Correctly handle indexes on virtual columns #2641
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Notably, there's a comment in the previous logic for adding columns with default/generated values: However, it appears that in fact the exact opposite is true: the column indexes computed during analysis of an |
nicktobey
force-pushed
the
nicktobey/indexed-virtual
branch
from
f74a998
to
0f9211a
Compare
…currently panics.
nicktobey
force-pushed
the
nicktobey/indexed-virtual
branch
from
0f9211a
to
5bea118
Compare
zachmu
approved these changes
Aug 27, 2024
| }, | ||
| }, | ||
| { | ||
| Name: "creating unique index on stored generated column", |
| { | ||
| Query: "select * from t1 where b = 2 order by a", | ||
| Expected: []sql.Row{{1, float64(2)}}, | ||
| Skip: true, // https://github.com/dolthub/dolt/issues/8276 |
| }, | ||
| }, | ||
| { | ||
| Name: "creating unique index on virtual generated column", |
| @@ -276,6 +276,14 @@ func (td *TableData) numRows(ctx *sql.Context) (uint64, error) { | |||
| // throws an error if any two or more rows share the same |cols| values. | |||
| func (td *TableData) errIfDuplicateEntryExist(cols []string, idxName string) error { | |||
| columnMapping, err := td.columnIndexes(cols) | |||
|
|
|||
| // We currently skip validating duplicates on unique virtual columns. | |||
| for _, i := range columnMapping { | |||
There was a problem hiding this comment.
Should have a skipped test assertion for this
There was a problem hiding this comment.
Done, and added comments.
Unfortunately the assertion will get skipped for both GMS and Dolt, but I don't think we have a way to just disable it for GMS.
This was referenced Aug 29, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes dolthub/dolt#8276
Lots of small behaviors around virtual columns were not working correctly:
This PR adds tests for these cases and fixes them by tweaking the logic for projections on tables with generated columns.