- Notifications
You must be signed in to change notification settings - Fork927
feat: Implement joins with golang templates#6429
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Removes the need to fetch the workspace to run an rbac check.
Uh oh!
There was an error while loading.Please reload this page.
| // "$1, $2" instead of "$1, $1". | ||
| // 2. SQLx does not handle uuid arrays. | ||
| // 3. SQLx only supports ":name" style arguments and breaks "::" type casting. | ||
| func bindNamed(query string, arg interface{}) (newQuery string, args []interface{}, err error) { |
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.
Is there no other way around this? I really dislike the regexp rewriting and reflection on every single query.
Is sqlx not smarter than this?
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.
Actually SQLx is a bit dummber. I took this code implementation from SQLx and changed some things. The usage for SQLx issqlx.Named(). But it doesn't handle things that I put in the function comments, so I adjusted it.
https://github.com/jmoiron/sqlx/blob/master/named.go#LL421-L436C2
Instead of a regex, they wrote a more proper parser:https://github.com/jmoiron/sqlx/blob/master/named.go#L331
But the parser doesn't work. It parses comments and they use:arg as arguments, so when we typecast with::, it's seen as escaping a colon literal. So our sql queries would have to do::::.
The SQLx parser is honestly really dumb with actual SQL.
Uh oh!
There was an error while loading.Please reload this page.
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.
Looks fine to me except forloadQueries, this should happen as close to startup as possible.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Golang template alternative to#6371
What this does
This adds database joins and dynamic SQL queries. I only did this for workspace builds in this PR. Read the other PR and this issue:#6426.
Fixes:#6426
Next work
I will use templates for authorized queries next, remove the strings replace.