[FLINK-39419][table] Switch TO_CHANGELOG to row semantics with full deletes + require update before#27911
[FLINK-39419][table] Switch TO_CHANGELOG to row semantics with full deletes + require update before#27911gustavodemorais wants to merge 4 commits intoapache:masterfrom
Conversation
gustavodemorais
changed the title
twalthr
left a comment
twalthr
left a comment
There was a problem hiding this comment.
Thanks for this simplification @gustavodemorais. This aligns better with existing concepts.
| // UPDATE_BEFORE and full DELETE rows. REQUIRE_FULL_DELETE is | ||
| // redundant without partition keys (isPtfUpsert is always false) | ||
| // but documents the intent explicitly. |
There was a problem hiding this comment.
| // UPDATE_BEFORE and full DELETE rows. REQUIRE_FULL_DELETE is | |
| // redundant without partition keys (isPtfUpsert is always false) | |
| // but documents the intent explicitly. | |
| // UPDATE_BEFORE and implicitly full DELETE rows. |
There was a problem hiding this comment.
I've added REQUIRE_FULL_DELETE in the PR just to explicitly state what we're doing, even though it's not necessary. I think that makes it more clear what we're doing, always generating full deletes
I've added a short note for that on top of REQUIRE_FULL_DELETE - I could also delete it, I don't have a strong opinion
| * different lengths (e.g., {@code CAST('INSERT' AS VARCHAR(12))}). | ||
| */ | ||
| private static SqlNode unwrapCast(final SqlNode node) { | ||
| if (node.getKind() == SqlKind.CAST) { |
There was a problem hiding this comment.
we also need to check the type of the cast, otherwise you might strip cast like cast(int as double) or cast(string as boolean)
| ToChangelogTestPrograms.LAG_ON_RETRACT_VIA_CHANGELOG, | ||
| ToChangelogTestPrograms.DELETION_FLAG, | ||
| ToChangelogTestPrograms.MISSING_PARTITION_BY, | ||
| ToChangelogTestPrograms.REJECTS_PARTITION_BY, |
There was a problem hiding this comment.
not necessary, already tested by ptf framework
…type, remove redundant test
|
Thanks for the review, @twalthr! I've addressed your feedback, take a look |
3 participants
What is the purpose of the change
Switches TO_CHANGELOG from SET_SEMANTIC_TABLE to ROW_SEMANTIC_TABLE, removing the PARTITION BY requirement. The planner inserts ChangelogNormalize for upsert sources to guarantee UPDATE_BEFORE and full DELETE rows (correctness over performance)..
Brief change log
Verifying this change
Does this pull request potentially affect one of the following parts:
@Public(Evolving): (no)Documentation