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: bindArray on SELECT from VALUES list #811
base: master
Are you sure you want to change the base?
Conversation
@jmoiron We are suffering a lot from this bug, please merge. |
@jmoiron +1 |
I'd also love to see this merged so #796 could be solved. |
@@ -224,7 +224,7 @@ func bindStruct(bindType int, query string, arg interface{}, m *reflectx.Mapper) | |||
return bound, arglist, nil | |||
} | |||
|
|||
var valuesReg = regexp.MustCompile(`\)\s*(?i)VALUES\s*\(`) | |||
var valuesReg = regexp.MustCompile(`[\)|\(]\s*(?i)VALUES\s*\(`) |
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.
I see test cases justifying )
and (
but not |
. Unless anyone wants to add a test demonstrating the usefulness of allowing a leading |
, maybe this PR could get unblocked by removing that option?
var valuesReg = regexp.MustCompile(`[\)|\(]\s*(?i)VALUES\s*\(`) | |
var valuesReg = regexp.MustCompile(`[)(]\s*(?i)VALUES\s*\(`) |
Note there is no need to escape parentheses within a character class, since capture groups cannot exist inside character classes.
fixBound
currently assumes named var in VALUES is only from INSERTbindArray
is not populating the right number of placeholdersIdeally the regex should be something like:
(?<!(?i)into)\W+(?i)VALUES\s*\(
but Golang regex doesn't support look around