Copilot commented on code in PR #7935:
URL: https://github.com/apache/ignite-3/pull/7935#discussion_r3038354093
##########
modules/sql-engine/src/integrationTest/java/org/apache/ignite/internal/sql/engine/ItFloatingPointTest.java:
##########
@@ -205,15 +205,15 @@ void testEqualityComparison() {
assertQuery("SELECT fn FROM test WHERE fn =
'-Infinity'::FLOAT").returns(Float.NEGATIVE_INFINITY).check();
assertQuery("SELECT f FROM test WHERE f =
'+Infinity'::FLOAT").returns(Float.POSITIVE_INFINITY).check();
assertQuery("SELECT fn FROM test WHERE fn =
'+Infinity'::FLOAT").returns(Float.POSITIVE_INFINITY).check();
- assertQuery("SELECT f FROM test WHERE f =
'NaN'::FLOAT").returnNothing().check(); // NaN never equals
- assertQuery("SELECT fn FROM test WHERE fn =
'NaN'::FLOAT").returnNothing().check(); // NaN never equals
+ assertQuery("SELECT f FROM test WHERE f =
'NaN'::FLOAT").returns(Float.NaN).check();
+ assertQuery("SELECT fn FROM test WHERE fn =
'NaN'::FLOAT").returns(Float.NaN).check();
assertQuery("SELECT d FROM test WHERE d =
'-Infinity'::DOUBLE").returns(Double.NEGATIVE_INFINITY).check();
assertQuery("SELECT dn FROM test WHERE dn =
'-Infinity'::DOUBLE").returns(Double.NEGATIVE_INFINITY).check();
assertQuery("SELECT d FROM test WHERE d =
'+Infinity'::DOUBLE").returns(Double.POSITIVE_INFINITY).check();
assertQuery("SELECT dn FROM test WHERE dn =
'+Infinity'::DOUBLE").returns(Double.POSITIVE_INFINITY).check();
- assertQuery("SELECT d FROM test WHERE d =
'NaN'::DOUBLE").returnNothing().check(); // NaN never equals
- assertQuery("SELECT dn FROM test WHERE dn =
'NaN'::DOUBLE").returnNothing().check(); // NaN never equals
+ assertQuery("SELECT d FROM test WHERE d =
'NaN'::DOUBLE").returns(Double.NaN).check();
+ assertQuery("SELECT dn FROM test WHERE dn =
'NaN'::DOUBLE").returns(Double.NaN).check();
Review Comment:
The equality assertions for NaN were updated to treat `NaN = NaN` as true,
but the later non-equality tests in this class still assume Java primitive NaN
ordering (e.g., `< NaN` returning no rows). With the new `compare()`-based
semantics, queries like `f < NaN` should return all non-NaN values (and similar
for DOUBLE), so those expectations need to be updated to keep the test suite
consistent.
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/exp/SqlExpressionFactoryImplTest.java:
##########
@@ -828,6 +829,132 @@ private static Stream<Arguments> numericLiterals() {
);
}
+ @ParameterizedTest
+ @MethodSource("doubleSpecialValuesArgs")
+ public void testDoubleSpecialValuesComparison(
+ SqlOperator op,
+ double leftVal,
+ double rightVal,
+ boolean expectedResult
+ ) {
+ RexBuilder rexBuilder = Commons.rexBuilder();
+ IgniteTypeFactory tf = Commons.typeFactory();
+
+ RelDataType colType = tf.createSqlType(SqlTypeName.DOUBLE);
+ RelDataType rowType = new Builder(tf)
+ .add("c1", colType)
+ .add("c2", colType)
+ .build();
+
+ RexInputRef ref1 = rexBuilder.makeInputRef(rowType, 0);
+ RexInputRef ref2 = rexBuilder.makeInputRef(rowType, 1);
+ RexNode filter = rexBuilder.makeCall(op, List.of(ref1, ref2));
+
+ SqlPredicate predicate = expFactory.predicate(filter, rowType);
+ assertEquals(expectedResult, predicate.test(ctx, new Object[]{leftVal,
rightVal}),
+ "Failed for " + leftVal + " " + op.getName() + " " + rightVal);
+ }
+
+ private static List<Arguments> doubleSpecialValuesArgs() {
+ return makeSpecialFloatArgs(
+ Double.NaN,
+ Double.POSITIVE_INFINITY,
+ Double.NEGATIVE_INFINITY,
+ 1.0d,
+ 0.0d
+ );
+ }
+
+ @ParameterizedTest
+ @MethodSource("floatSpecialValuesArgs")
+ public void testFloatSpecialValues(
+ SqlOperator op,
+ float left,
+ float right,
+ boolean exoected
+ ) {
+ RexBuilder rexBuilder = Commons.rexBuilder();
+ IgniteTypeFactory tf = Commons.typeFactory();
+
+ RelDataType colType = tf.createSqlType(SqlTypeName.REAL);
+ RelDataType rowType = new Builder(tf)
+ .add("c1", colType)
+ .add("c2", colType)
+ .build();
+
+ RexInputRef ref1 = rexBuilder.makeInputRef(rowType, 0);
+ RexInputRef ref2 = rexBuilder.makeInputRef(rowType, 1);
+ RexNode filter = rexBuilder.makeCall(op, List.of(ref1, ref2));
+
+ SqlPredicate predicate = expFactory.predicate(filter, rowType);
+ assertEquals(exoected, predicate.test(ctx, new Object[]{left, right}),
+ "Failed for " + left + " " + op.getName() + " " + right);
Review Comment:
In `testFloatSpecialValues`, the parameter name `exoected` is a typo and
makes the assertion harder to read/maintain. Rename it to `expected` (and
update its usage in the assertion).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
