[
https://issues.apache.org/jira/browse/FLINK-41423?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Gustavo de Morais updated FLINK-41423:
--------------------------------------
Description:
{{SinkUpsertMaterializer}} matches records by upsert key with a generated
{{RecordEqualiser}} ({{SinkMaterializeUpsertKeyEqualiser}} in
{{StreamExecSink}}). Unless both rows are {{BinaryRowData}}, the equaliser
compares fields with SQL {{=}} semantics ({{EqualiserCodeGenerator}}):
DOUBLE/FLOAT with {{==}} ({{-0.0 = 0.0}}), MAP and MULTISET ignoring the entry
order, ROW field by field. Keyed state and shuffles compare bytes, so two
different upsert keys are treated as the same record and a delete for one
removes the other.
Reproducer on master, no join needed:
{code:sql}
CREATE TABLE r (id INT NOT NULL, k DOUBLE NOT NULL, v STRING NOT NULL, name
STRING,
PRIMARY KEY (id, k) NOT ENFORCED) WITH ('connector' = 'values',
'changelog-mode' = 'I,UB,UA,D');
CREATE TABLE sink (id INT NOT NULL, k DOUBLE NOT NULL, v STRING NOT NULL, name
STRING,
PRIMARY KEY (id, v) NOT ENFORCED) WITH ('connector' = 'values',
'sink-changelog-mode-enforced' = 'I,UA,D');
INSERT INTO sink SELECT id, k, v, name FROM r ON CONFLICT DO DEDUPLICATE;
{code}
Input: {{+I(1, 0.0, v, r1)}}, {{+I(1, -0.0, v, r2)}}, {{-D(1, 0.0, v, r1)}}.
Expected sink content: {{+I(1, -0.0, v, r2)}}. Actual: empty. The same happens
with {{k}} as {{ROW<d DOUBLE>}} ({{ROW(0.0)}} vs {{ROW(-0.0)}}), {{MAP<INT,
STRING>}} (the same entries in a different order) and {{MULTISET<STRING>}}.
With keys that differ under {{=}} too, e.g. {{ROW(1.0)}} vs {{ROW(2.0)}},
{{r2}} is kept as expected.
FLINK-38753 makes more queries reach this, since it finds upsert keys in more
plans.
We should compare upsert keys byte-wise in the materializer, consistent with
keyed state. {{SinkUpsertMaterializer}}, {{SinkUpsertMaterializerV2}} and
{{WatermarkCompactingSinkMaterializer}} all compare the key through a
{{ProjectedRowData}} view with the equaliser from {{StreamExecSink}}, so a
byte-wise upsert key equaliser generated there (e.g. projecting the key into a
{{BinaryRowData}}) fixes all three. {{EqualiserCodeGenerator}} itself should
stay as is, since many other operators use it.
This should be backwards compatible: the equaliser is not part of the compiled
plan, and V1 and the watermark materializer keep full rows in state.
{{SinkUpsertMaterializerV2}} stores the generated equaliser and hash function
in the {{RowDataKey}} serializer snapshot, but restore uses the new instances,
so a restore test should confirm it. V2 is likely already byte-wise on RocksDB,
where {{MapState}} keys are compared serialized, and only affected on the heap
backend.
was:
{{SinkUpsertMaterializer}} matches records by upsert key with a generated
{{RecordEqualiser}} ({{SinkMaterializeUpsertKeyEqualiser}} in
{{StreamExecSink}}). Unless both rows are {{BinaryRowData}}, the equaliser
compares fields with SQL {{=}} semantics ({{EqualiserCodeGenerator}}):
DOUBLE/FLOAT with {{==}} ({{-0.0 = 0.0}}), MAP and MULTISET ignoring the entry
order, ROW field by field. Keyed state and shuffles compare bytes, so two
different upsert keys are treated as the same record and a delete for one
removes the other.
Reproducer on master, no join needed:
{code:sql}
CREATE TABLE r (id INT NOT NULL, k DOUBLE NOT NULL, v STRING NOT NULL, name
STRING,
PRIMARY KEY (id, k) NOT ENFORCED) WITH ('connector' = 'values',
'changelog-mode' = 'I,UB,UA,D');
CREATE TABLE sink (id INT NOT NULL, k DOUBLE NOT NULL, v STRING NOT NULL, name
STRING,
PRIMARY KEY (id, v) NOT ENFORCED) WITH ('connector' = 'values',
'sink-changelog-mode-enforced' = 'I,UA,D');
INSERT INTO sink SELECT id, k, v, name FROM r ON CONFLICT DO DEDUPLICATE;
{code}
Input: {{+I(1, 0.0, v, r1)}}, {{+I(1, -0.0, v, r2)}}, {{-D(1, 0.0, v, r1)}}.
Expected sink content: {{+I(1, -0.0, v, r2)}}. Actual: empty. The same happens
with {{k}} as {{ROW<d DOUBLE>}} ({{ROW(0.0)}} vs {{ROW(-0.0)}}), {{MAP<INT,
STRING>}} (the same entries in a different order) and {{MULTISET<STRING>}}.
With keys that differ under {{=}} too, e.g. {{ROW(1.0)}} vs {{ROW(2.0)}},
{{r2}} is kept as expected.
FLINK-38753 makes more queries reach this, since it finds upsert keys in more
plans.
We should compare upsert keys byte-wise in the materializer, consistent with
keyed state.
> SinkUpsertMaterializer compares upsert keys with SQL equality instead of
> bytes and drops rows
> ---------------------------------------------------------------------------------------------
>
> Key: FLINK-41423
> URL: https://issues.apache.org/jira/browse/FLINK-41423
> Project: Flink
> Issue Type: Bug
> Components: Table SQL / Planner, Table SQL / Runtime
> Reporter: Gustavo de Morais
> Priority: Major
>
> {{SinkUpsertMaterializer}} matches records by upsert key with a generated
> {{RecordEqualiser}} ({{SinkMaterializeUpsertKeyEqualiser}} in
> {{StreamExecSink}}). Unless both rows are {{BinaryRowData}}, the equaliser
> compares fields with SQL {{=}} semantics ({{EqualiserCodeGenerator}}):
> DOUBLE/FLOAT with {{==}} ({{-0.0 = 0.0}}), MAP and MULTISET ignoring the
> entry order, ROW field by field. Keyed state and shuffles compare bytes, so
> two different upsert keys are treated as the same record and a delete for one
> removes the other.
> Reproducer on master, no join needed:
> {code:sql}
> CREATE TABLE r (id INT NOT NULL, k DOUBLE NOT NULL, v STRING NOT NULL, name
> STRING,
> PRIMARY KEY (id, k) NOT ENFORCED) WITH ('connector' = 'values',
> 'changelog-mode' = 'I,UB,UA,D');
> CREATE TABLE sink (id INT NOT NULL, k DOUBLE NOT NULL, v STRING NOT NULL,
> name STRING,
> PRIMARY KEY (id, v) NOT ENFORCED) WITH ('connector' = 'values',
> 'sink-changelog-mode-enforced' = 'I,UA,D');
> INSERT INTO sink SELECT id, k, v, name FROM r ON CONFLICT DO DEDUPLICATE;
> {code}
> Input: {{+I(1, 0.0, v, r1)}}, {{+I(1, -0.0, v, r2)}}, {{-D(1, 0.0, v, r1)}}.
> Expected sink content: {{+I(1, -0.0, v, r2)}}. Actual: empty. The same
> happens with {{k}} as {{ROW<d DOUBLE>}} ({{ROW(0.0)}} vs {{ROW(-0.0)}}),
> {{MAP<INT, STRING>}} (the same entries in a different order) and
> {{MULTISET<STRING>}}. With keys that differ under {{=}} too, e.g.
> {{ROW(1.0)}} vs {{ROW(2.0)}}, {{r2}} is kept as expected.
> FLINK-38753 makes more queries reach this, since it finds upsert keys in more
> plans.
> We should compare upsert keys byte-wise in the materializer, consistent with
> keyed state. {{SinkUpsertMaterializer}}, {{SinkUpsertMaterializerV2}} and
> {{WatermarkCompactingSinkMaterializer}} all compare the key through a
> {{ProjectedRowData}} view with the equaliser from {{StreamExecSink}}, so a
> byte-wise upsert key equaliser generated there (e.g. projecting the key into
> a {{BinaryRowData}}) fixes all three. {{EqualiserCodeGenerator}} itself
> should stay as is, since many other operators use it.
> This should be backwards compatible: the equaliser is not part of the
> compiled plan, and V1 and the watermark materializer keep full rows in state.
> {{SinkUpsertMaterializerV2}} stores the generated equaliser and hash function
> in the {{RowDataKey}} serializer snapshot, but restore uses the new
> instances, so a restore test should confirm it. V2 is likely already
> byte-wise on RocksDB, where {{MapState}} keys are compared serialized, and
> only affected on the heap backend.
--
This message was sent by Atlassian Jira
(v10.3.26#10030026)
