You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@asterixdb.apache.org by ti...@apache.org on 2018/11/03 00:52:13 UTC
asterixdb git commit: [ASTERIXDB-2469][TEST] Tests Can Result In
False Positives For Numericals
Repository: asterixdb
Updated Branches:
refs/heads/master e9a911f5c -> a993e9bf8
[ASTERIXDB-2469][TEST] Tests Can Result In False Positives For Numericals
- user model changes: no
- storage format changes: no
- interface changes: no
Details:
- The test framework doesn’t differentiate between numeric types in
expected results.
- With the current behavior, when comparing numerical cases,
as a last step, we try to convert both numbers to double and
do a comparison (in case the String camparison failed already),
this is good for the cases of having something like: expected 100.0
and acutal 10E1 (for whatever reason), in this case the String comparison
will fail, but the double conversion will produce the correct result.
- With this change, we ensure the above behavior is maintained,
however, we check that the types are compatible first, for example,
if the expected is 100.0 and the actual is 100.0, this should succeed,
but if the expected is 100.0 and the actual is 100, then this will fail,
this way we ensure correctness of both the numerical value as well as
correctness of data type check.
Change-Id: I918b7e5c3c39271f77a7d5a01ff634c2a0221ebc
Reviewed-on: https://asterix-gerrit.ics.uci.edu/3009
Reviewed-by: Till Westmann <ti...@apache.org>
Sonar-Qube: Jenkins <je...@fulliautomatix.ics.uci.edu>
Tested-by: Jenkins <je...@fulliautomatix.ics.uci.edu>
Contrib: Jenkins <je...@fulliautomatix.ics.uci.edu>
Integration-Tests: Jenkins <je...@fulliautomatix.ics.uci.edu>
Project: http://git-wip-us.apache.org/repos/asf/asterixdb/repo
Commit: http://git-wip-us.apache.org/repos/asf/asterixdb/commit/a993e9bf
Tree: http://git-wip-us.apache.org/repos/asf/asterixdb/tree/a993e9bf
Diff: http://git-wip-us.apache.org/repos/asf/asterixdb/diff/a993e9bf
Branch: refs/heads/master
Commit: a993e9bf8263481b1c57546cbe58461ef6e0ab5f
Parents: e9a911f
Author: Hussain Towaileb <Hu...@Gmail.com>
Authored: Tue Oct 30 14:48:01 2018 +0300
Committer: Till Westmann <ti...@apache.org>
Committed: Fri Nov 2 17:51:46 2018 -0700
----------------------------------------------------------------------
.../asterix/test/common/TestExecutor.java | 32 ++++++++------
.../ensure_result_numeric_type.3.query.sqlpp | 30 +++++++++++++
.../ensure_result_numeric_type.1.adm | 1 +
.../resources/runtimets/testsuite_sqlpp.xml | 7 +++
.../runtime/evaluators/common/NumberUtils.java | 45 ++++++++++++++++++++
5 files changed, 102 insertions(+), 13 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/a993e9bf/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
index 98cd668..113fc67 100644
--- a/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
+++ b/asterixdb/asterix-app/src/test/java/org/apache/asterix/test/common/TestExecutor.java
@@ -64,6 +64,7 @@ import org.apache.asterix.app.external.IExternalUDFLibrarian;
import org.apache.asterix.common.api.Duration;
import org.apache.asterix.common.config.GlobalConfig;
import org.apache.asterix.common.utils.Servlets;
+import org.apache.asterix.runtime.evaluators.common.NumberUtils;
import org.apache.asterix.test.server.ITestServer;
import org.apache.asterix.test.server.TestServerProvider;
import org.apache.asterix.testframework.context.TestCaseContext;
@@ -373,25 +374,30 @@ public class TestExecutor {
}
return false;
} else {
- // If the fields are floating-point numbers, test them
- // for equality safely
+ // Get the String values
expectedFields[j] = expectedFields[j].split(",")[0];
actualFields[j] = actualFields[j].split(",")[0];
- try {
- Double double1 = Double.parseDouble(expectedFields[j]);
- Double double2 = Double.parseDouble(actualFields[j]);
- float float1 = (float) double1.doubleValue();
- float float2 = (float) double2.doubleValue();
- if (Math.abs(float1 - float2) == 0) {
- continue;
- } else {
+ // Ensure type compatibility before value comparison
+ if (NumberUtils.isSameTypeNumericStrings(expectedFields[j], actualFields[j])) {
+ try {
+ Double double1 = Double.parseDouble(expectedFields[j]);
+ Double double2 = Double.parseDouble(actualFields[j]);
+ float float1 = (float) double1.doubleValue();
+ float float2 = (float) double2.doubleValue();
+
+ if (Math.abs(float1 - float2) == 0) {
+ continue;
+ } else {
+ return false;
+ }
+ } catch (NumberFormatException ignored) {
+ // Guess they weren't numbers - must simply not be equal
return false;
}
- } catch (NumberFormatException ignored) {
- // Guess they weren't numbers - must simply not be equal
- return false;
}
+
+ return false; // Not equal
}
}
}
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/a993e9bf/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
new file mode 100644
index 0000000..3933e66
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/misc/ensure_result_numeric_type/ensure_result_numeric_type.3.query.sqlpp
@@ -0,0 +1,30 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+ /*
+ * This test is expected to return a ComparisonException
+ * because the actual returned value is 3 (int) but the expected
+ * value is 3.0 (double)
+ */
+
+
+select element strict_count((
+ select element x
+ from [1,2,3] as x
+));
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/a993e9bf/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
new file mode 100644
index 0000000..9f55b2c
--- /dev/null
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/results/misc/ensure_result_numeric_type/ensure_result_numeric_type.1.adm
@@ -0,0 +1 @@
+3.0
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/a993e9bf/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
index 591d441..18455df 100644
--- a/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
+++ b/asterixdb/asterix-app/src/test/resources/runtimets/testsuite_sqlpp.xml
@@ -4582,6 +4582,13 @@
</compilation-unit>
</test-case>
<test-case FilePath="misc">
+ <compilation-unit name="ensure_result_numeric_type">
+ <output-dir compare="Text">ensure_result_numeric_type</output-dir>
+ <source-location>false</source-location>
+ <expected-error>expected < 3.0</expected-error>
+ </compilation-unit>
+ </test-case>
+ <test-case FilePath="misc">
<compilation-unit name="partition-by-nonexistent-field">
<output-dir compare="Text">partition-by-nonexistent-field</output-dir>
<expected-error>Field "id" is not found</expected-error>
http://git-wip-us.apache.org/repos/asf/asterixdb/blob/a993e9bf/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/NumberUtils.java
----------------------------------------------------------------------
diff --git a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/NumberUtils.java b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/NumberUtils.java
index 0b2e94c..5fd7892 100644
--- a/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/NumberUtils.java
+++ b/asterixdb/asterix-runtime/src/main/java/org/apache/asterix/runtime/evaluators/common/NumberUtils.java
@@ -116,4 +116,49 @@ public final class NumberUtils {
private NumberUtils() {
}
+
+ /**
+ * Checks if 2 strings are numeric and of the same type
+ * @param value1 first string value
+ * @param value2 second string value
+ * @return {@code true} if value1 and value2 are numeric values and of the same type, {@code false} otherwise
+ */
+ public static boolean isSameTypeNumericStrings(String value1, String value2) {
+ // Step 1: Confirm numeric strings
+ if (isNumericString(value1) && isNumericString(value2)) {
+ // Step 2: Confirm same type (2 ints or 2 floats = true, otherwise false)
+ return isIntegerNumericString(value1) == isIntegerNumericString(value2);
+ }
+
+ // Not numeric string
+ return false;
+ }
+
+ /**
+ * Checks if a string is a numeric value
+ * @param value string to be checked
+ * @return {@code true} if the string is a valid number, {@code false} otherwise
+ */
+ public static boolean isNumericString(String value) {
+ try {
+ Double.parseDouble(value);
+ return true;
+ } catch (NumberFormatException ignored) {
+ return false;
+ }
+ }
+
+ /**
+ * Checks if a numeric string is of type int
+ * @param value numeric string value
+ * @return {@code true} if the string is of type int, {@code false} otherwise
+ */
+ public static boolean isIntegerNumericString(String value) {
+ try {
+ Integer.parseInt(value);
+ return true;
+ } catch (NumberFormatException ignored) {
+ return false;
+ }
+ }
}