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

fix insert for subquery aliases with on duplicate update expr #2855

Merged
merged 5 commits into from
Feb 20, 2025
Merged

Conversation

jycor
Copy link
Contributor

@jycor jycor commented Feb 20, 2025

The table not found error was actually a bad error message; it really wasn't finding the column due to some weird logic around resolving duplicate indexes.

The logic assumes that n.Destination.Schema() and n.Source.Schema() must be equal length, and if it's not then we need to replace the Source columns' Source node with the provided OnDupValuesPrefix.

Additionally, when assigning getfield indexes, we can't assume that if the schema lengths are mismatched that they must be using the OnDupValuesPrefix. Instead, we should always search for a matching column, and then substitute.

fixes: dolthub/dolt#8862

Copy link
Contributor

@max-hoffman max-hoffman left a comment

Choose a reason for hiding this comment

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

I think this looks good, given my present understanding of how insert values indexing works

if values, ok := n.Source.(*plan.Values); ok {
// The source table is either named via VALUES(...) AS ... or has
// the default value planbuilder.OnDupValuesPrefix
destSch := n.Destination.Schema()
Copy link
Contributor

Choose a reason for hiding this comment

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

i've gone through line by line and i'm maybe missing if there was any logical change, correct me if i'm wrong

continue
}
if i < len(srcScope.cols) {
combinedScope.newColumn(srcScope.cols[i])
Copy link
Contributor

Choose a reason for hiding this comment

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

this is the only change? not putting the extra cols in combined scope?

@jycor jycor merged commit f785905 into main Feb 20, 2025
8 checks passed
@jycor jycor deleted the james/insert branch February 20, 2025 20:09
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.

derived table syntax not supported (in select with insert into?)
2 participants