Hi everyone,
I'd like to start a discussion about changing the default of
"table.exec.sink.require-on-conflict" from "true" to "false" for 2.4. The
ON CONFLICT functionality stays as is. This is only about whether users
must explicitly provide the clause.
Since 2.3 [1], a query fails at planning time when its upsert key differs
from the sink's primary key and it has no ON CONFLICT clause. After
collecting feedback from users, clients and tooling developers, I think
making the clause opt-in would be a better default for the following
reasons:
1. The obvious pain: upgrades break working queries. A query that ran
fine on 2.2 can fail on 2.3 with no change to the SQL. We've seen a lot of
confusion and pain from users, clients and tools migrating from 2.2 to 2.3,
and we could avoid that for future migrations by changing the default.
2. The planner can't always prove the keys match. Upsert key inference
is conservative. Functions, casts or projections can drop the key even when
the query is correct, and the user gets an error for something the planner
couldn't work out. We're working on improving that, but there are still
cases where the engine can't preserve the keys.
3. The clause only exists on INSERT INTO. CTAS, RTAS and CREATE
MATERIALIZED TABLE have no way to set it except a session config. This
makes the breaking change inconsistent, and users can't move a query from
INSERT INTO to CTAS or a materialized table without falling back to that
config. We should first make the clause available for all of them.
Without the clause, Flink would deduplicate like it did before 2.3. Users
who already added ON CONFLICT to their queries don't need to change
anything.
I want to be clear that I think the clause is very useful, and I'd still
recommend requiring it for setups that mostly use INSERT INTO and where
users understand deduplication and its effects. But as long as the clause
is only available for INSERT INTO and not for every statement that writes
to a table, requiring it by default doesn't feel right.
Looking forward to hearing the community's opinion!
[1]
https://cwiki.apache.org/confluence/display/FLINK/FLIP-558:+Improvements+to+SinkUpsertMaterializer+and+changelog+disorder
Kind regards,
Gustavo
