Support incremental_strategy parameter and new insert_overwrite strategy#2195
Support incremental_strategy parameter and new insert_overwrite strategy#2195SuchodolskiEdvin wants to merge 2 commits into
Conversation
SuchodolskiEdvin
commented
Jun 2, 2026
SuchodolskiEdvin
commented
- updated proto with new parameters
- added new tests
- added validation for chosen incremental_strategies
- added new insert_overwrite strategy logic
SuchodolskiEdvin
force-pushed
the
insert_overwrite_incremental_strategy
branch
from
9f72a8c to
9724071
Compare
SuchodolskiEdvin
force-pushed
the
insert_overwrite_incremental_strategy
branch
from
9724071 to
41d152d
Compare
| MERGE ${resolveTargetTable} T | ||
| USING \`${stagingTableUnqualified}\` S | ||
| ON FALSE | ||
| WHEN NOT MATCHED BY SOURCE AND ${partitionBy} IN UNNEST(partitions_for_replacement) ${updatePartitionFilter ? `and T.${updatePartitionFilter}` : ""} THEN |
There was a problem hiding this comment.
Such code T.${updatePartitionFilter} will only work when updatePartitionFilter has exactly one expression?
There was a problem hiding this comment.
Yes, you are correct. It is a known limitation that T.${updatePartitionFilter} only works for simple expressions (and fails on multi-expression SQL). Current implementation is designed to match the existing behavior of the standard MERGE strategy to maintain consistency between the two strategies for now. The fix of using updatePartitionFilter with several expression will be introduced in a separate PR.
There was a problem hiding this comment.
I think it's better to implement it correctly from the start. IIRC current behaviour for MERGE may require an opt-in flag to avoid breaking existing code, so I'd avoid such compatibility issues.
SuchodolskiEdvin
force-pushed
the
insert_overwrite_incremental_strategy
branch
4 times, most recently
from
f11f8f3 to
1942a72
Compare
| case dataform.IncrementalStrategy.INSERT_OVERWRITE: | ||
| if (!this.proto.bigquery || !this.proto.bigquery.partitionBy) { | ||
| this.session.compileError( | ||
| new Error("incrementalStrategy 'insert_overwrite' requires 'partitionBy' to be set."), |
There was a problem hiding this comment.
Nit: Inconsistent style. in some places we have capital first letter and in others we don't. We also use "." in some places and miss it in others. for example on line 759.
| ...baseTable, | ||
| incrementalStrategy: dataform.IncrementalStrategy.INSERT_OVERWRITE, | ||
| bigquery: { | ||
| partitionBy: "DATE(ts)", |
There was a problem hiding this comment.
Do we have an example column that is not a function like Date. My concern is that we a re biased toward handling this method. so it its just a normal column name we might not handle it correctly.
There was a problem hiding this comment.
I added additional test for normal column.
SuchodolskiEdvin
force-pushed
the
insert_overwrite_incremental_strategy
branch
3 times, most recently
from
a90b434 to
8f1bcb7
Compare
| MERGE ${resolveTargetTable} T | ||
| USING \`${stagingTableUnqualified}\` S | ||
| ON FALSE | ||
| WHEN NOT MATCHED BY SOURCE AND ${partitionBy} IN UNNEST(partitions_for_replacement) ${updatePartitionFilter ? `and T.${updatePartitionFilter}` : ""} THEN |
There was a problem hiding this comment.
I think it's better to implement it correctly from the start. IIRC current behaviour for MERGE may require an opt-in flag to avoid breaking existing code, so I'd avoid such compatibility issues.
- updated proto with new parameters - added new tests - added validation for chosen incremental_strategies - added new insert_overwrite strategy logic
SuchodolskiEdvin
force-pushed
the
insert_overwrite_incremental_strategy
branch
2 times, most recently
from
d5b89f1 to
eb6ca9b
Compare
- Replaced legacy 'T' and 'S' MERGE aliases with explicit 'DATAFORM_DEST' and 'DATAFORM_SOURCE' in the BigQuery adapter. - Updated `incremental_table_test.ts` assertions. - Documented the new `incrementalPredicates` property in the OSS API reference.
SuchodolskiEdvin
force-pushed
the
insert_overwrite_incremental_strategy
branch
from
eb6ca9b to
cc9eb94
Compare
3 participants
