raminqaf opened a new pull request, #29481:
URL: https://github.com/apache/flink/pull/29481
## What is the purpose of the change
Comparing two MAPs or MULTISETs with `=` returns FALSE when they hold equal
values, because the generated code compares the boxed values by reference.
```sql
-- f0 and f1 both hold {'a': 1.5, 'b': 2.5} as MAP<STRING, DOUBLE>
SELECT f0 = f1 FROM t
```
| Equal values in both maps | Before | After |
|----------------------------------------------------|--------|-------|
| `MAP<STRING, DOUBLE>` `{a: 1.5, b: 2.5}` | FALSE | TRUE |
| `MAP<STRING, INT>` `{a: 7, b: 5000}` | FALSE | TRUE |
| `MAP<STRING, DATE>` `{a: 1985-11-04, b: 2018-07-26}` | FALSE | TRUE |
| `MULTISET<STRING>` `{a: 200, b: 300}` | FALSE | TRUE |
`generateMapComparison` passes the boxed map values to `generateEquals`,
which emits `==` for numeric, DATE, TIME and interval types. INT values only
compare correctly inside the Integer cache. The fix unboxes the values first,
so `=` on map values behaves like `=` on the scalar type. For DOUBLE that means
NaN is not equal to NaN and 0.0 equals -0.0. The null handling is unchanged.
I did not switch to `.equals()`. `Double.equals` compares bits, so it would
disagree with `DOUBLE =` on NaN and 0.0.
The same code is used by `IS [NOT] DISTINCT FROM` and by the field
comparison of the `RecordEqualiser`. Rows with MAP or MULTISET fields therefore
also compare by value in the stateful operators that use the equaliser, for
example deduplication and group aggregation. Those operators now skip updates
that only differ in the identity of a box.
Not part of this PR:
- BINARY and UUID map keys still compare by array identity.
- ARRAY and ROW over binary data still take a byte comparison shortcut that
depends on the entry order.
- A map with a NULL value still compares unequal to every map, including a
copy of itself.
## Brief change log
- `ScalarOperatorGens#generateMapComparison` unboxes the map values with
`generateInputFieldUnboxing` before it calls `generateEquals`.
- `MapFunctionITCase` gets equality cases for maps and multisets.
## Verifying this change
This change added tests and can be verified as follows:
- `MapFunctionITCase` has new equality cases for DOUBLE, INT, DATE,
DECIMAL, nested maps, maps with different keys or size, NULL values, MULTISET
with counts above 127 and maps built from columns. Each case checks `=` and
`<>` in the Table API and in SQL.
- Without the fix, 18 of the 27 new test items fail with `expected:
+I[true] but was: +I[false]`. The DECIMAL, different keys or size and NULL
cases pass either way.
- `CollectionFunctionsITCase`, `CastFunctionITCase`,
`JsonFunctionsITCase`, `ComparisonFunctionITCase`, `ScalarOperatorsTest` and
`EqualiserCodeGeneratorTest` pass.
## Does this pull request potentially affect one of the following parts:
- Dependencies (does it add or upgrade a dependency): no
- The public API, i.e., is any changed class annotated with
`@Public(Evolving)`: no
- The serializers: no
- The runtime per-record code paths (performance sensitive): yes. The
generated map comparison adds a null check and an assignment per compared value.
- Anything that affects deployment or recovery: JobManager (and its
components), Checkpointing, Kubernetes/Yarn, ZooKeeper: no
- The S3 file system connector: no
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
---
##### Was generative AI tooling used to co-author this PR?
- [X] Yes (please specify the tool below)
Generated-by: Claude Code 2.1.299 (Claude Sonnet 5.5)
--
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]
