fix(db): Resolve the execution table through search_path in the revision guard (backport #929) - #933
Merged
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Advanced Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
…ion guard (backport #929) The guard backported in #910 tested information_schema against current_schema(), which is only the first entry in search_path. On a connection whose search_path puts another schema first, an execution table in a later schema makes the predicate false while the unqualified ALTER below it resolves to that same later table, so the widening is skipped without a word and execution.revision silently stays integer. Resolve the relation with to_regclass and read its type from pg_attribute, so the guard and the ALTER always agree on which table they are talking about. to_regclass returns NULL when no execution relation is reachable, which leaves the guard false as before. Two narrower cases came with it. to_regclass resolves any relation, so the guard is restricted to tables by relkind: an index named execution earlier in search_path would otherwise match a pg_attribute row and send ALTER TABLE at something it cannot alter, failing the whole file. And the column type is compared after resolving a domain to its base, so a column already typed as a domain over bigint keeps the domain and its CHECK constraints instead of having them stripped by the ALTER. Covered by two tests against a real Postgres: execution in a schema that is not first in search_path is widened and its rows preserved on two consecutive applies, and a non-table relation of that name leaves the apply clean rather than aborting it. Signed-off-by: Prasanth Baskar <prasanth@8gears.com>
bupd
force-pushed
the
backport/pr-929-to-release-2.15
branch
from
September 16, 2026 08:03
823ea25 to
b042b85
Compare
Contributor
|
Preview images for this PR are available in
Verify a preview image: Verify SBOM attestation: |
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
1 participant
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.
Manual backport of #929 to
release-2.15. A clean cherry-pick did not apply, becauserelease-2.15had noexecution.revisionblock at all when this was cut.Order is already settled: #910 merged first, so this is now a pure delta on top of it — it changes the guard #910 introduced and adds the regression tests. Nothing here duplicates #910.
What broke
The guard #910 brought over tests
information_schemaagainstcurrent_schema(), which is only the first entry insearch_path. If a connection'ssearch_pathputs another schema first andexecutionlives in a later one, the guard reads false while the unqualifiedALTER TABLE executionbelow it resolves to that later table. The widening is skipped silently andexecution.revisionstaysinteger.What changed
Resolve the relation with
to_regclassand read its type frompg_attribute, so the guard and theALTERalways agree on which table they mean. Two narrower cases came with it:relkindkeeps the guard on tables.to_regclassresolves any relation, so an index namedexecutionearlier insearch_pathwould otherwise match apg_attributerow and sendALTER TABLEat something it cannot alter — aborting the whole file.bigintkeeps the domain and its CHECK constraints instead of having them stripped.How it was verified
Against a
release-2.15database built by its own numbered migrations, not a fixture. Seededrevision=99, then appliedharbor_next.sqltwice the way core does on start:The widening fires, the existing value survives it, and the repeat is a no-op.
Two regression tests ported from #929, both green on this branch — one putting
executionin a schema that is not first insearch_path, one with a non-table relation of that name shadowing it:Schema rules respected: no new numbered migration, the change stays in the idempotent
harbor_next.sql.Note on the first run
The first push here showed only lint/DCO/bot checks, which looked like a missing DB lane. It was not: that head conflicted with
release-2.15, and a conflicted PR has no merge ref forpull_requestworkflows to build, so everything needing one was skipped. The rebase onto the merged #910 resolved the conflict and the full set (Test, Build, Go Lint, Vulnerability Check, preview images) now runs.