-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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 generated column in unique key error #1461
Conversation
Copilot
AI
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Files not reviewed (2)
- localtests/generated-columns-unique/create.sql: Language not supported
- localtests/generated-columns/create.sql: Language not supported
Comments suppressed due to low confidence (1)
go/sql/builder_test.go:691
- The test case should include a scenario where the unique key columns contain virtual columns to ensure the new logic is covered.
_, err := NewDMLUpdateQueryBuilder(databaseName, tableName, tableColumns, sharedColumns, sharedColumns, uniqueKeyColumns)
Tip: If you use Visual Studio Code, you can request a review from Copilot before you push from the "Source Control" tab. Learn more
@@ -244,6 +245,19 @@ func (this *ColumnList) IsSubsetOf(other *ColumnList) bool { | |||
return true | |||
} | |||
|
|||
func (this *ColumnList) FilterBy(f func(Column) bool) *ColumnList { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're creating a new instance of ColumnList
, what if we turn this method into factory function? Something like:
NewFilteredColumnList(ordinals ColumnsMap, f func(Column) bool) *ColumnList {
...
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really I just want to filter []Column
, but IsSubsetOf
is a method of ColumnList
so I had to make a new one 😓
Description
This PR fixes a bug when the unique key contains a virtual column. The update query builder checks
uniqueKeyColumns
is a subset ofsharedColumns
which only contains non-virtual columns, so the migration fails.We can reproduce the bug by adding
update
DML to the generated-columns-unique test case.script/cibuild
returns with no formatting errors, build errors or unit test errors.