Handle parsing of quoted identifiers #39
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
crisptrutski
commented
May 24, 2024
src/macaw/rewrite.clj
Outdated
| (when-let [schema-rename (find schema-renames (.getSchemaName t))] | ||
| (vswap! updated-nodes conj [(.getSchemaName t) schema-rename]) | ||
| (.setSchemaName t (val schema-rename)))) | ||
| (let [schema-name (collect/normalize-reference (.getSchemaName t) _ctx)] |
There was a problem hiding this comment.
A miss from the previous PR, but I don't think it's worth cherry-picking.
crisptrutski
force-pushed
the
handle-quotes
branch
from
fefb3e1
to
5c3da72
Compare
piranha
reviewed
May 28, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refs #34
Before this change, inconsistent use of quotes could cause issues with reading and rewriting the identifiers within SQL strings. With this change they will be deduplicated when scanning, and matched when replacing.
In keeping with the dialect agnosticism of the JSQLParser grammar, we simply handle the two most common quotes for now. Many dialects event support both of them. In future we may allow the set of allowed quotes to be passed in as an optional parameter.
The next quote I'd like to support is
[, which is interesting for two reasons - 1. the closing brace is different, and 2. the aforementioned grammar does not currently support them (🙀). We could work around this by find-replacing them with supported braces, tracking their original positions with entries inreplacements, or we might want to fix the grammar. It's a pity perhaps that the parser itself doesn't treat quotes as a first-class AST citizen, that might also simplify things. The replacement technique however would extend nicely to 3rd party drivers requiring further quotes.There's some overlap with our case insensitivity logic, as some databases are case sensitive for quoted identifiers only. This creates the need for another optional flag. We should probably document all these public flags we're accumulating.
I decided to not bother with implicit re-quoting where necessary - for now we can leave it up to library users to quote their replacement values where necessary. This way we can remain more database agnostic for now.