fix: allow repeated alter table in explicit transactions - #26610
Conversation
Qodo reviews are paused for this user.Troubleshooting steps vary by plan Learn more → On a Teams plan? Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center? |
XuPeng-SH
left a comment
XuPeng-SH
left a comment
There was a problem hiding this comment.
Reviewed the transaction/snapshot lineage change end to end. The fix correctly distinguishes live logical branch ownership from historical ALTER lineage, preserves the fixed snapshot for autocommit, and uses the transaction view when explicit transaction workspace history must remain visible. The DAG traversal is cycle-safe and covers ancestor/descendant ownership. Focused tests for component ownership, fixed-snapshot selection, SQL generation, and historical-only explicit transactions pass; go vet and diff checks pass. The full affected package run has three parquet fanout failures, all reproduced with the same assertions on the clean base commit, so they are pre-existing and unrelated. CI is green at this exact head.
Merge Queue Status
This pull request spent 39 minutes 53 seconds in the queue, with no time running CI. Waiting for
All conditions
ReasonPull request #26610 has been dequeued Pull request from fork cannot be queued. This pull request comes from a fork, and Mergify needs the author's permission to update its branch.
Failing checks:
HintYou should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it. Tick the box to put this pull request back in the merge queue (same as
|
4 participants
What type of PR is this?
Which issue(s) this PR fixes:
Fixes #26549
What this PR does / why we need it:
Root cause
COPY-based
ALTER TABLErecordslevel='alter'metadata when a snapshot or PITR must retain an old physical table generation. That metadata is historical ownership, but the explicit-transaction guard treated any lineage participation as a live logical data branch. Consequently, the second ordinaryALTER TABLEin the same transaction was rejected with a data-branch error.There was a second correctness constraint behind that failure: after advancing a pessimistic transaction snapshot, an explicit
{MO_TS}copy cannot see the transaction workspace. Using it for repeated ALTER would lose earlier transaction DML or fail to resolve the physical generation created by an earlier ALTER.Changes
tableoralter:table). Traversing the full component preserves the restriction when a live branch exists through an ancestor or sibling.BEGIN,autocommit=0, DML before ALTER, repeated add/modify/rename operations, indexed and base scans, rollback, database snapshots with interleaved tables, and negative controls for real data branches.Compatibility and safety
The change does not alter metadata or on-disk formats. Explicit-transaction ALTER remains rejected for real data branches, including an altered branch and an altered base table connected to a live sibling branch.
Validation
ALTER on a data-branch lineage is not supported inside an explicit transaction, then passed with this change.go testpassed for the focused regressions and the completepkg/frontend/databranchutilsandpkg/sql/compilepackages.go build -mod=readonly ./pkg/frontend/databranchutils ./pkg/sql/compilego vet -mod=readonly ./pkg/frontend/databranchutils ./pkg/sql/compilemake buildalter_table_historical_lineage: 3 consecutive runs, each75/75passed.diff_schema_evolve:278/278passed.