Skip to content

Fix dependency-name computation in schema-apply ordering#12

Merged
aersam merged 1 commit into
mainfrom
drop-sqlglot-c-extra
Jul 17, 2026
Merged

Fix dependency-name computation in schema-apply ordering#12
aersam merged 1 commit into
mainfrom
drop-sqlglot-c-extra

Conversation

@aersam

@aersam aersam commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Two real bugs in how _iter_sql_files/_get_sql_deps compute the name a file is tracked under, both silently degrading dependency ordering to "untracked -> assumed already satisfied" instead of an actual wait:

  • _get_sql_deps used str(t) on a matched Table node, which includes the alias for an aliased reference (e.g. "FROM app.visit v" -> "app.visit AS v"). That never matches the plain name a dependency's own CREATE target is recorded under. Use exp.table_name(t) instead.
  • _iter_sql_files derived a file's own declared name from the raw file stem, so a numeric ordering prefix (e.g. "0_visit_unified.sql") landed in all_declared as "schema.0_visit_unified" instead of "schema.visit_unified" -- never matching what any dependent file's SQL actually references. Strip it the same way the schema-directory name already is.

apply_schema()'s retry-until-no-progress loop (this version) still converges even with these bugs present, since it retries raw SQL execution directly rather than relying on this bookkeeping - but the computed ordering itself was wrong, causing avoidable extra retry passes.

Two real bugs in how _iter_sql_files/_get_sql_deps compute the name a
file is tracked under, both silently degrading dependency ordering to
"untracked -> assumed already satisfied" instead of an actual wait:

- _get_sql_deps used str(t) on a matched Table node, which includes the
  alias for an aliased reference (e.g. "FROM app.visit v" -> "app.visit
  AS v"). That never matches the plain name a dependency's own CREATE
  target is recorded under. Use exp.table_name(t) instead.
- _iter_sql_files derived a file's own declared name from the raw file
  stem, so a numeric ordering prefix (e.g. "0_visit_unified.sql") landed
  in all_declared as "schema.0_visit_unified" instead of
  "schema.visit_unified" -- never matching what any dependent file's SQL
  actually references. Strip it the same way the schema-directory name
  already is.

apply_schema()'s retry-until-no-progress loop (this version) still
converges even with these bugs present, since it retries raw SQL
execution directly rather than relying on this bookkeeping - but the
computed ordering itself was wrong, causing avoidable extra retry passes.
@aersam
aersam merged commit edd15c4 into main Jul 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant