perf: honor incremental_apply_config_changes flag in V1 incremental path - #1467
Merged
sd-db merged 3 commits intoJun 23, 2026
Merged
Conversation
moomindani
force-pushed
the
perf/v1-incremental-apply-config-changes-flag
branch
from
June 10, 2026 23:52
1246066 to
45f2a3c
Compare
The V1 incremental merge path called adapter.get_relation_config() unconditionally, forcing metadata fetches (tags, column_tags, constraints, column_masks, tblproperties, describe_extended) on every incremental run even when the user opted out via incremental_apply_config_changes=false. The V2 path already honors the flag via the process_config_changes() macro; V1 ignored it. Wrap the V1 get_relation_config call in a flag check matching the V2 semantics. When the flag is false, _configuration_changes stays None and the existing apply-block guard (`if _configuration_changes is not none`) short-circuits the apply, so no metadata is fetched or applied. Default behavior (flag=true) is unchanged. Co-authored-by: Isaac
moomindani
force-pushed
the
perf/v1-incremental-apply-config-changes-flag
branch
from
June 13, 2026 01:57
45f2a3c to
df0d8cc
Compare
Collaborator
|
/integration-test |
|
Integration tests dispatched for PR #1467 by @sd-db. Track progress in the Actions tab. |
Coverage report
This PR does not seem to contain any modification to coverable code. |
|
Integration results for PR #1467 — UC cluster ✅ success · SQL warehouse ❌ failure · All-purpose cluster ✅ success · Shard coverage ✅ success |
|
Integration results for PR #1467 — UC cluster ✅ success · SQL warehouse ✅ success · All-purpose cluster ✅ success · Shard coverage ✅ success |
sd-db
approved these changes
Jun 23, 2026
sd-db
left a comment
sd-db
left a comment
Collaborator
There was a problem hiding this comment.
Changes look good, thanks for the PR !!
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves #1402
Follow-up to closed #1403 with a much smaller surface area, in response to @sd-db's earlier review.
Description
The V1 incremental merge path calls
adapter.get_relation_config()unconditionally during every incremental run, forcing metadata diff queries (tags, column_tags, constraints, column_masks, tblproperties, describe_extended) on every model. The V2 path already honorsincremental_apply_config_changes: falsevia theprocess_config_changes()macro; V1 ignored the flag.#1387 partially addressed this by auto-skipping
fetch_tags/fetch_column_tagswhen those configs are absent. This PR is complementary, not duplicate:requires_server_metadata_for_diff()get_relation_configChange: wrap the V1
get_relation_configcall in anif config.get('incremental_apply_config_changes', True)block. Whenfalse,_configuration_changesstaysnoneand the existing apply-block guard short-circuits the apply. Default behavior unchanged.Security: the V1 inline apply block never pulled
column_masksfrom the changeset to begin with — socolumn_masksare not applied in V1 today, and remain not applied with this change. The flag set tofalsestrictly reduces work; no new application path is introduced. Theapply_column_masksV2-only guard from the final commit of #1403 is therefore not needed in this scoped change.Diff size: +6 / -3 in
incremental.sql, plus CHANGELOG and one functional test.Test plan
Checklist