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

Insert VALUES statements with subqueries index correctly #2263

Merged
merged 2 commits into from
Jan 16, 2024

Conversation

max-hoffman
Copy link
Contributor

@max-hoffman max-hoffman commented Jan 16, 2024

fixes: dolthub/dolt#7322

A couple things I noticed in the process of debugging this:

  • We appear to inline inject an empty row when evaluating VALUES statements. Code comments suggest TRIGGERS are the reason for this. Most of the time this doesn't impact other value statements, and I think we've adapted VALUE expressions to use this pattern, but it shifts subquery expression indexes in a way that we were not accounting for. The change here updates subqueries in INSERT VALUES to expect this offset.
  • We can't fix this during expression indexing at the INSERT level, because a second analysis of VALUES statements is necessary, for unclear reasons.
  • We differentiate between INSERT VALUES and INSERT SELECTs, because INSERT SELECTs do not inject an offset row. In theory we could probably normalize these two cases and inject an empty row into insert select sources, but I ran into problems trying to implement the fix this way.

Copy link
Member

@zachmu zachmu left a comment

Choose a reason for hiding this comment

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

LGTM

},
Assertions: []ScriptTestAssertion{
{
Query: "INSERT INTO uv(v, x_id) VALUES ('test', (SELECT x FROM xy WHERE y = 'admin'));",
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to have a test of an error case here too -- what if there is more than one result from the subquery?

@max-hoffman max-hoffman merged commit 9df15d5 into main Jan 16, 2024
7 checks passed
@max-hoffman max-hoffman deleted the max/values-scalar-subq branch January 16, 2024 21:59
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.

Subquery in an INSERT statement return NULL
2 participants