Skip to content

Go: Fix database upgrade script - #22783

Merged
owen-mc merged 2 commits into
github:mainfrom
owen-mc:go/fix/range-stmt-upgrade-script
Oct 8, 2026
Merged

owen-mc merged 2 commits into
github:mainfrom
owen-mc:go/fix/range-stmt-upgrade-script

Conversation

@owen-mc

@owen-mc owen-mc commented Oct 8, 2026

Copy link
Copy Markdown
Contributor

In #22182 a change was made to the dbsceme for range loops. The upgrade script (in 171046a) was a no-op and was marked as partial compatibility. It should have been marked as breaking compatibility. Instead, this PR actually makes a full compatibility upgrade script. I have tested this locally and it worked as expected. I have also done careful manual review that this reproduces the missing relations that the new version of the extractor creates.

@owen-mc
owen-mc requested a review from a team as a code owner October 8, 2026 11:21
@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label Oct 8, 2026
Copilot AI balanced review requested due to automatic review settings October 8, 2026 11:21
@github-actions github-actions Bot added the Go label Oct 8, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

The upgrade accurately reproduces the extractor’s new relations and parent structure.

0 open findings

What changed in this PR

Adds a complete Go database upgrade for synthesized range-loop element expressions.

Changes:

  • Synthesizes range-element nodes, reparents loop variables, and copies locations.
  • Marks the upgrade as fully compatible and configures relation transformations.
File Description
upgrade.ql Reconstructs extractor-equivalent range-element data.
upgrade.properties Runs transformations and declares full compatibility.

🧠 Review effort: Balanced


Give feedback about Copilot approvals in this survey to enter a drawing for a $150 gift card.

@owen-mc
owen-mc requested a review from a team October 8, 2026 11:56
Comment thread go/ql/lib/upgrades/5ff5325d274ae4f86defa195577bc7c1370b72fa/upgrade.ql Outdated
idx in [0 .. 1]
}

query predicate new_exprs(NewExpr id, int kind, NewExprParent parent, int idx) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With the upgrade script, the parent/child relation is expanded
Before

range_stmt ->_0 expr_0
range_stmt ->_1 expr_1

After

range_stm ->_0 range_expression
range_expression ->_0 expr_0
range_expression ->_1 expr_1

Perhaps, a downgrade script is needed as well that contracts the parent/child relation (and deletes the range_expression's).

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was already done in the original commit.

@michaelnebel michaelnebel left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@owen-mc
owen-mc merged commit 2a29eaf into github:main Oct 8, 2026
18 checks passed
@owen-mc
owen-mc deleted the go/fix/range-stmt-upgrade-script branch October 8, 2026 13:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Go no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants