raminqaf commented on code in PR #29466:
URL: https://github.com/apache/flink/pull/29466#discussion_r4261392335
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionITCase.java:
##########
@@ -1155,7 +1162,42 @@ private static List<TestSetSpec> variantRowCasts() {
FIELD(
"user",
ROW(FIELD("id", INT()),
FIELD("since", STRING()))),
- FIELD("tags", ARRAY(STRING())))));
+ FIELD("tags", ARRAY(STRING())))),
+ // A ROW cast from VARIANT compares field by field, like a ROW
built with ROW(...):
+ // NaN never equals NaN, and 0.0 equals -0.0. PARSE_JSON
rejects NaN and
+ // infinities, so each VARIANT is built from a row of strings.
Each pair spells its
+ // value differently, because the planner merges equal inputs
and folds x = x to
+ // TRUE.
+ TestSetSpec.forExpression("Compare ROW values cast from
VARIANT")
+ .onFieldsWithData(
+ Row.of("NaN"),
+ Row.of("nan"),
+ Row.of("0.0"),
+ Row.of("-0.0"),
+ Row.of("Infinity"),
+ Row.of("inf"),
+ Row.of("-Infinity"))
+ .andDataTypes(
+ stringRow, stringRow, stringRow, stringRow,
stringRow, stringRow,
+ stringRow)
+ // NaN = NaN
+ .testSqlResult(variantRowsEqual("f0", "f1"), false,
BOOLEAN())
Review Comment:
I ran a test with `WordCountSqlExample` reading from a CSV file so nothing
gets folded. Here is the result with the left and right side being `NaN` or
`0.0` and `-0.0`
| How both sides are built (`l` and `r`) |
NaN = NaN | 0.0 = -0.0 |
|---------------------------------------------------------|-----------|------------|
| `l = r` on two DOUBLE columns | FALSE |
TRUE |
| `ROW(l) = ROW(r)` inline, which Calcite rewrites to `l = r` | FALSE |
TRUE |
| `ROW<d DOUBLE>` columns read from the CSV source | FALSE |
TRUE |
| `ARRAY[l] = ARRAY[r]`, a BinaryArrayData | TRUE |
FALSE |
| `ROW cast from VARIANT` on master, a BinaryRowData (Before) | TRUE
| FALSE |
| `ROW cast from VARIANT` **in this PR**, a GenericRowData | FALSE |
TRUE |
So it depends on the row class, not the type. The generated
`RecordEqualiser` compares two `BinaryRowData` byte by byte, and any other rows
field by field with `==`. `ARRAY` has the same shortcut for `BinaryArrayData`.
Two NaN values have the same bytes, so the byte comparison says TRUE. `0.0` and
`-0.0` have different bytes, so it says FALSE.
`DOUBLE =` compiles to Java's `==`, and in Java `Double.NaN == Double.NaN`
is false. That follows IEEE 754. A NaN can come from very different
calculations, such as `sqrt(-1)` and `inf - inf`, so two NaNs should not count
as equal.
This PR moves the VARIANT cast to the field-by-field side, which matches
`DOUBLE =`. The inconsistency itself predates this PR, so I would fix the byte
shortcut in a separate ticket.
--
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]
