feat(generator)!: Fix the json path with single quote issue - #8309
feat(generator)!: Fix the json path with single quote issue#8309fivetran-amrutabhimsenayachit wants to merge 3 commits into
Conversation
SQLGlot Integration Test Results✅ All tests passedComparing:
Overallmain: 182937 total, 163862 passed (pass rate: 89.6%) sqlglot:fix_json_path_with_single_quote: 182937 total, 163862 passed (pass rate: 89.6%) Transitions: ✅ All tests passed |
|
@fivetran-amrutabhimsenayachit same feedback re: semver as I shared here. Also, let's simplify descriptions even more; it still feels wall-of-text-ish due to the long sentences. Doesn't help with review, I just skip it instinctively. |
| if self._quote_json_path_key_using_brackets and self.JSON_PATH_SINGLE_QUOTE_ESCAPE: | ||
| escaped = expression.replace("'", "\\'") | ||
| escaped = f"\\'{expression}\\'" | ||
| escaped = f"\\'{escaped}\\'" |
There was a problem hiding this comment.
This should be escaped = f"'{escaped}'", right?
There was a problem hiding this comment.
If we remove the backslashes there, we stop escaping the inner single quotes at the jsonpath layer. That changes the generated sql text.
Eg:
$[\'it\'s\'] will be changed to $['it\'s']
There was a problem hiding this comment.
Yeap. we can also remove entirly the https://github.com/tobymao/sqlglot/pull/8309/changes#diff-362498362ae539e216cd83ebfe510977107648742f1a1bee7b2db7b5b7d27974R5404-R5407
There was a problem hiding this comment.
Removing this entirely would cause other dialects like Postgres, SQLite, MySQL, Presto, Trino, Snowflake, DuckDB, ClickHouse, and Redshift to break. As this is literally the fix provided for the issue reported in #8251
Sure, changed the scope to generator. For this particular PR, I just summarized the description from the actual ticket:#8251, which has all the details about the issue. |
fixes #8251
Problem
When a JSON key contains an apostrophe (e.g
. Customer's dept), sqlglot generated broken SQL — the apostrophe closed the SQL string early, producing text that couldn't even be parsed back, let alone run on a real database. On top of that, BigQuery's olderJSON_EXTRACT/JSON_EXTRACT_SCALARfunctions reject such keys even when properly escaped — only itsJSON_VALUE/JSON_QUERYfunctions actually work.Fix
Escape the apostrophe correctly wherever a JSON key gets wrapped in a SQL string, across all affected dialects (Postgres, SQLite, MySQL, BigQuery, Databricks, etc.) — so the generated SQL is always valid and parses back cleanly.
For BigQuery specifically, automatically switch
JSON_EXTRACT/JSON_EXTRACT_SCALAR/JSON_EXTRACT_ARRAYtoJSON_QUERY/JSON_VALUE/JSON_QUERY_ARRAYwhenever the key has an apostrophe, since real BigQuery rejects the old functions for that case regardless of escaping.Test Summary: