Copilot commented on code in PR #41492:
URL: https://github.com/apache/superset/pull/41492#discussion_r3679077504
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:
##########
@@ -176,8 +196,8 @@ export function useKeywords(
}
let { caption } = data;
- if (data.meta === 'table' && caption.includes(' ')) {
- caption = `"${caption}"`;
+ if (data.meta === 'table') {
+ caption = quoteIdentifier(caption, identifierQuote);
}
Review Comment:
`caption` is treated as always present, but
`AceCompleterKeywordData.caption` is optional and some keyword entries don’t
set it. If `caption` is missing, `quoteIdentifier(caption, ...)` can crash or
insert `undefined`. Consider falling back to `name`/`value`/empty string before
quoting.
##########
tests/unit_tests/db_engine_specs/test_base.py:
##########
@@ -1383,3 +1383,11 @@ def
test_base_spec_public_information_includes_supports_offset() -> None:
assert "supports_offset" in info
assert info["supports_offset"] is True
+
+
+def test_get_public_information_exposes_ansi_identifier_quote() -> None:
+ """The base spec advertises ANSI double quotes for identifier quoting."""
+ assert BaseEngineSpec.get_public_information()["identifier_quote"] == {
+ "start": '"',
+ "end": '"',
+ }
Review Comment:
`BaseEngineSpec` is referenced without being imported in this test, which
will raise a `NameError` at runtime.
--
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]
