Support incremental_strategy parameter and new insert_overwrite strategy by SuchodolskiEdvin · Pull Request #2195 · dataform-co/dataform

SuchodolskiEdvin · 2026-06-02T14:39:33Z

updated proto with new parameters
added new tests
added validation for chosen incremental_strategies
added new insert_overwrite strategy logic

kolina · 2026-06-07T20:30:48Z

+  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


Such code T.${updatePartitionFilter} will only work when updatePartitionFilter has exactly one expression?

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.

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.

Tuseeq1 · 2026-06-12T08:44:53Z

+      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."),


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.

Tuseeq1 · 2026-06-12T09:20:55Z

+      ...baseTable,
+      incrementalStrategy: dataform.IncrementalStrategy.INSERT_OVERWRITE,
+      bigquery: {
+        partitionBy: "DATE(ts)",


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.

I added additional test for normal column.

kolina · 2026-06-27T01:00:55Z

+  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


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.

kolina · 2026-07-07T03:06:25Z

        ":node_modules",
        "//packages/@dataform/core:package_tar",
-        "//test_credentials:bigquery.json",
+        # "//test_credentials:bigquery.json",


Please merge 3478e82 into your PR and uncomment

Hm, but it's still commented (so if tests are passing, it may not be needed and can be removed)

- updated proto with new parameters - added new tests - added validation for chosen incremental_strategies - added new insert_overwrite strategy logic

- 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 · 2026-07-10T10:52:38Z

@@ -134,6 +134,7 @@ Action config defines the contents of `actions.yaml` configuration files.
 | partitionExpirationDays      | [int32](#int32)                                                                                                                    |          | The number of days for which BigQuery stores data in each partition. The setting applies to all partitions in a table, but is calculated independently for each partition based on the partition time.                          |
 | requirePartitionFilter       | [bool](#bool)                                                                                                                      |          | Declares whether the partitioned table requires a WHERE clause predicate filter that filters the partitioning column.                                                                                                           |
 | updatePartitionFilter        | [string](#string)                                                                                                                  |          | SQL-based filter for when incremental updates are applied.                                                                                                                                                                      |


@kolina Should I mark this field as Deprecated?

You'll need to add support for this field in SQL generation in GCP code before deprecating the old field.

kolina · 2026-07-11T20:37:08Z

+      incrementalStrategy: dataform.IncrementalStrategy.INSERT_OVERWRITE,
+      bigquery: {
+        partitionBy: "DATE(ts)",
+        incrementalPredicates: ["ts >= '2024-01-01'"]


Let's have use table aliases here to showcase how to use it

kolina · 2026-07-11T20:38:37Z

+    ) {
+      this.session.compileError(
+        new Error(
+          `IncrementalPredicates and updatePartitionFilter cannot be both set. Use only incrementalPredicates.`


incrementalPredicates at the start of sentence?

kolina · 2026-07-11T20:39:46Z

        ":node_modules",
        "//packages/@dataform/core:package_tar",
-        "//test_credentials:bigquery.json",
+        # "//test_credentials:bigquery.json",


Hm, but it's still commented (so if tests are passing, it may not be needed and can be removed)

kolina · 2026-07-11T20:41:32Z

@@ -134,6 +134,7 @@ Action config defines the contents of `actions.yaml` configuration files.
 | partitionExpirationDays      | [int32](#int32)                                                                                                                    |          | The number of days for which BigQuery stores data in each partition. The setting applies to all partitions in a table, but is calculated independently for each partition based on the partition time.                          |
 | requirePartitionFilter       | [bool](#bool)                                                                                                                      |          | Declares whether the partitioned table requires a WHERE clause predicate filter that filters the partitioning column.                                                                                                           |
 | updatePartitionFilter        | [string](#string)                                                                                                                  |          | SQL-based filter for when incremental updates are applied.                                                                                                                                                                      |


You'll need to add support for this field in SQL generation in GCP code before deprecating the old field.

SuchodolskiEdvin assigned kolina and SuchodolskiEdvin Jun 2, 2026

SuchodolskiEdvin requested a review from a team as a code owner June 2, 2026 14:39

SuchodolskiEdvin requested review from zaptot and removed request for a team June 2, 2026 14:39

SuchodolskiEdvin force-pushed the insert_overwrite_incremental_strategy branch from 9f72a8c to 9724071 Compare June 2, 2026 15:13

SuchodolskiEdvin requested review from kolina and removed request for zaptot June 3, 2026 09:56

SuchodolskiEdvin force-pushed the insert_overwrite_incremental_strategy branch from 9724071 to 41d152d Compare June 3, 2026 13:46

kolina requested changes Jun 7, 2026

View reviewed changes

SuchodolskiEdvin force-pushed the insert_overwrite_incremental_strategy branch 4 times, most recently from f11f8f3 to 1942a72 Compare June 8, 2026 16:29

SuchodolskiEdvin requested review from Tuseeq1 and kolina June 10, 2026 09:04

Tuseeq1 requested changes Jun 12, 2026

View reviewed changes

SuchodolskiEdvin force-pushed the insert_overwrite_incremental_strategy branch 3 times, most recently from a90b434 to 8f1bcb7 Compare June 22, 2026 16:03

SuchodolskiEdvin requested a review from Tuseeq1 June 22, 2026 16:30

Tuseeq1 approved these changes Jun 23, 2026

View reviewed changes

kolina requested changes Jun 27, 2026

View reviewed changes

SuchodolskiEdvin force-pushed the insert_overwrite_incremental_strategy branch 3 times, most recently from eb6ca9b to cc9eb94 Compare July 3, 2026 09:01

SuchodolskiEdvin requested a review from kolina July 3, 2026 09:06

kolina requested changes Jul 7, 2026

View reviewed changes

SuchodolskiEdvin added 2 commits July 9, 2026 13:04

Support incremental_strategy parameter and new insert_overwrite strategy

a2df8b9

- 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 cc9eb94 to 222cfc4 Compare July 10, 2026 10:26

SuchodolskiEdvin requested a review from kolina July 10, 2026 10:33

SuchodolskiEdvin commented Jul 10, 2026

View reviewed changes

kolina approved these changes Jul 11, 2026

View reviewed changes

Uh oh!

Conversation

SuchodolskiEdvin commented Jun 2, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants