-
Notifications
You must be signed in to change notification settings - Fork 1
Support attribute exists #125
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
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
simonrw
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.
Thanks for making this improvement!
In general I am not a fan of us writing our own character-to-character SQL generator in this library, so I'd love to hear your thoughts on where we go from here.
| env: | ||
| LOCALSTACK_AUTH_TOKEN: ${{ secrets.LOCALSTACK_API_KEY }} | ||
| TEST_IMAGE_NAME: public.ecr.aws/lambda/nodejs:16 | ||
| TEST_IMAGE_NAME: public.ecr.aws/lambda/nodejs:18 |
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.
Er... we are still using 16 (for better or worse...) https://github.com/localstack/localstack-ext/blob/1f952f304e39d1d066a4d703a2727b395bf8af9e/localstack-pro-core/localstack/pro/core/services/appsync/mapping.py#L25
However we do want to move our JS execution to the node in the container image, so this is a good move, thanks! 👍
| case "notContains": | ||
| return `${startGrouping}${this.quoteChar}${columnName}${this.quoteChar} NOT LIKE ${value}${endGrouping}`; | ||
| case "attributeExists": | ||
| return `${startGrouping}${this.quoteChar}${columnName}${this.quoteChar} IS ${value? "NOT " : ""}NULL${endGrouping}`; |
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'm not a super fan of this ternary operator, but it's quite common in JS so I'm ok with it.
Motivation
This pr implements support for
attributeExistsin rdswhereclause.The customer sample was also failing as part of the logic in the
whereclause was missing.Changes
attributeExistsor, processandor process a single clause. But only the first of these items would actually make it part of the SQL query.and/orclause. We are callingbuildWhereClauserecursively to correctly build nestedand/orclauses.buildWhereClausenow correctly returns a string. All usage ofbuildWhereClausewere adding it in a string literal. This would create invalid SQL statement if an array is returned.Other fixes along the way
Dateusage in some test, as they would fail in a non UTC environment