rusackas commented on code in PR #41492:
URL: https://github.com/apache/superset/pull/41492#discussion_r3693950555
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:
##########
@@ -54,19 +55,28 @@ const getHelperText = (value: string) =>
};
// Names that aren't simple identifiers (spaces, punctuation, leading digits)
-// must be double-quoted to be valid SQL, with embedded quotes doubled.
+// must be quoted to be valid SQL. The quote characters are dialect-specific
+// (e.g. ANSI double quotes, MySQL/MariaDB backticks, SQL Server square
brackets)
+// and are provided by the backend's database engine spec via
`engine_information`
+// so the mapping isn't duplicated here. Embedded quote characters are escaped
by
+// doubling the closing character.
+type IdentifierQuote = { start: string; end: string };
+const ANSI_QUOTE: IdentifierQuote = { start: '"', end: '"' };
const SIMPLE_IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]*$/;
-const quoteIdentifier = (identifier: string) =>
+const quoteIdentifier = (
+ identifier: string,
+ { start, end }: IdentifierQuote = ANSI_QUOTE,
+) =>
SIMPLE_IDENTIFIER_RE.test(identifier)
? identifier
- : `"${identifier.replace(/"/g, '""')}"`;
+ : `${start}${identifier.split(end).join(`${end}${end}`)}${end}`;
Review Comment:
Good catch, fixed. Added an `identifier_quote_escape_by_doubling` flag on
the engine spec (defaults True, BigQuery overrides False), so `quoteIdentifier`
picks backslash escaping for BigQuery instead of assuming doubling for every
backtick dialect.
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:
##########
@@ -54,19 +55,28 @@ const getHelperText = (value: string) =>
};
// Names that aren't simple identifiers (spaces, punctuation, leading digits)
-// must be double-quoted to be valid SQL, with embedded quotes doubled.
+// must be quoted to be valid SQL. The quote characters are dialect-specific
+// (e.g. ANSI double quotes, MySQL/MariaDB backticks, SQL Server square
brackets)
+// and are provided by the backend's database engine spec via
`engine_information`
+// so the mapping isn't duplicated here. Embedded quote characters are escaped
by
+// doubling the closing character.
+type IdentifierQuote = { start: string; end: string };
+const ANSI_QUOTE: IdentifierQuote = { start: '"', end: '"' };
const SIMPLE_IDENTIFIER_RE = /^[A-Za-z_][A-Za-z0-9_]*$/;
-const quoteIdentifier = (identifier: string) =>
+const quoteIdentifier = (
+ identifier: string,
+ { start, end }: IdentifierQuote = ANSI_QUOTE,
+) =>
SIMPLE_IDENTIFIER_RE.test(identifier)
? identifier
- : `"${identifier.replace(/"/g, '""')}"`;
+ : `${start}${identifier.split(end).join(`${end}${end}`)}${end}`;
Review Comment:
Yep, fixed the same way as the reply to @sadpandajoe above.
`escape_by_doubling` now comes from the engine spec (False for BigQuery), so
`quoteIdentifier` no longer assumes every backtick dialect escapes the same way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
