Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sql: add the missing tests to the planning changes around FOR UPDATE #19676

Closed
knz opened this issue Oct 31, 2017 · 1 comment
Closed

sql: add the missing tests to the planning changes around FOR UPDATE #19676

knz opened this issue Oct 31, 2017 · 1 comment
Assignees

Comments

@knz
Copy link
Contributor

knz commented Oct 31, 2017

PR #19650 has added planning support for FOR UPDATE, propagating the flag throughout the data source hierarchy.

However the planning changes in that PR were not complemented by tests that would check the flag indeed propagates to those table scans involved in the SELECT clause, and not to other tables.

This should be tested via EXPLAIN.

Hence the following suggestion:

  1. store the bit in the scanNode (even though the intents are not laid yet)
  2. have the plan walker (sql/walk.go) reveal that bit
  3. add EXPLAIN tests that show what happens for 1) simple queries 2) UPDATE/DELETE etc 3) with a join 4) with a join where only a subquery specifies the flag, e.g. select ... from (select ... for update), (select ...)
@rytaft
Copy link
Collaborator

rytaft commented Nov 14, 2017

PR #19650 was reverted with PR #20024. It was decided that SELECT FOR UPDATE support would be postponed in PR #19577. Closing this issue.

@rytaft rytaft closed this as completed Nov 14, 2017
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

No branches or pull requests

2 participants