rusackas commented on code in PR #41492:
URL: https://github.com/apache/superset/pull/41492#discussion_r3664775205
##########
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:
This is pre-existing behavior in `SIMPLE_IDENTIFIER_RE`, not something this
PR touches. Quoting reserved-word table names properly would need a per-dialect
keyword list, which feels like its own separate fix rather than folding into
this quoting-chars change.
--
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]
