-
Notifications
You must be signed in to change notification settings - Fork 871
Dynamic Queries #2859
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
base: main
Are you sure you want to change the base?
Dynamic Queries #2859
Conversation
// If the query string was edited, make sure the syntax is valid | ||
if expanded != rawSQL { | ||
if _, err := c.parser.Parse(strings.NewReader(expanded)); err != nil { | ||
return nil, fmt.Errorf("edited query syntax is invalid: %w", err) | ||
return nil, fmt.Errorf("edited query syntax is invalid: %w - %s", err, expanded) |
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.
This was to help me debug I could remove if you want.
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.
Should I move this pgx directory into a subdirectory with the stdlib directory?
@@ -1,3 +1,4 @@ | |||
{ | |||
"process": "sqlc-gen-json", |
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 didn't have sqlc-gen-json in my path so I added this to get the tests to pass I can remove it.
"dollar": func() bool { | ||
return req.Settings.Engine == string(config.EnginePostgreSQL) | ||
}, | ||
"hasDynamic": func() bool { |
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 wasn't sure how to add these values without putting them here. So while it does work it is kind of a hack.
We are unlikely to merge anything related to "dynamic" SQL until we have come up with a proposal internally. That said, I think we could look at the issue you mentioned related to table scope. Is there an open issue in the issue tracker for that? |
Yeah I contributed to the dynamic discussion and decided to create this pull request in order to showcase how it would work.
|
out = append(out, "pq.Array("+escape(v.Name)+")") | ||
} else { | ||
out = append(out, escape(v.Name)) | ||
} | ||
} else { | ||
for _, f := range v.Struct.Fields { | ||
if !f.HasSqlcSlice() && strings.HasPrefix(f.Type, "[]") && f.Type != "[]byte" && !v.SQLDriver.IsPGX() { | ||
if f.HasSqlcDynamic() { |
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.
Hi @ovadbar there is no logic inside this condition. May be it is not required?
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.
Maybe I have should have written it as the following. The code is needed
if !f.HasSqlcDynamic() {
if !f.HasSqlcSlice() && strings.HasPrefix(f.Type, "[]") && f.Type != "[]byte" && !v.SQLDriver.IsPGX() {
out = append(out, "pq.Array("+escape(v.VariableForField(f))+")")
} else {
out = append(out, escape(v.VariableForField(f)))
}
}
This is an attempt at my proposal for sqlc dynamic queries. (It also fixes an issue regarding table scope that I want).