You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "jesperct (via GitHub)" <gi...@apache.org> on 2026/06/18 13:20:20 UTC
[PR] fix(sqllab): quote autocomplete table names that need it [superset]
jesperct opened a new pull request, #41199:
URL: https://github.com/apache/superset/pull/41199
### SUMMARY
SQL Lab autocomplete inserted table names exactly as stored. When a name isn't a simple identifier (it contains spaces or punctuation, or starts with a digit), inserting it verbatim produces invalid SQL. Picking a table like `COVID Vaccines` from the dropdown gave `SELECT * FROM COVID Vaccines`, which fails to parse.
This quotes the inserted value with double quotes when the name isn't a simple identifier, doubling any embedded quotes so names containing `"` stay valid. Simple identifiers are still inserted as-is, and the human-readable label shown in the dropdown is unchanged.
Quoting uses ANSI double-quote identifiers. Dialect-specific quote characters (e.g. backticks on MySQL) would be a reasonable follow-up.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!-- before/after attached below -->
### TESTING INSTRUCTIONS
1. In SQL Lab, connect to a database/schema that has a table whose name needs quoting (for example, a name containing a space such as `COVID Vaccines`).
2. Start typing the table name in the editor and pick it from the autocomplete dropdown.
- Before: the name is inserted unquoted (`FROM COVID Vaccines`) and the query fails to parse.
- After: the name is inserted quoted (`FROM "COVID Vaccines"`) and runs.
3. Confirm a normal table name (e.g. `simple_table`) is still inserted without quotes.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41199:
URL: https://github.com/apache/superset/pull/41199#discussion_r3453015510
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:
##########
@@ -197,7 +205,7 @@ export function useKeywords(
() =>
allCachedTables.map(({ value, label, schema: tableSchema }) => ({
name: label,
- value,
+ value: quoteIdentifier(value),
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> Do not flag quoted table completion values in the Ace editor path when `toAceKeyword` strips `completer`; in that flow `insertMatch` does not run, so autocomplete insertion does not call `addTable` or store quoted names in table state/API calls.
**Applied to:**
- `superset-frontend/src/SqlLab/components/AceEditorProvider.tsx`
- `superset-frontend/src/SqlLab/components/EditorWrapper/**`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4770938029
**Bito Automatic Review Failed - Technical Failure** <br/><br/> Bito encountered technical difficulties while generating code feedback . To retry, type `/review` in a comment and save. If the issue persists, contact support@bito.ai and provide the following details: <br/><br/>Agent Run ID: 8d2afd9c-682b-46a7-98f5-afb4fa569347
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "jesperct (via GitHub)" <gi...@apache.org>.
jesperct commented on code in PR #41199:
URL: https://github.com/apache/superset/pull/41199#discussion_r3453010660
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:
##########
@@ -197,7 +205,7 @@ export function useKeywords(
() =>
allCachedTables.map(({ value, label, schema: tableSchema }) => ({
name: label,
- value,
+ value: quoteIdentifier(value),
Review Comment:
This does not apply on the Ace editor path. `toAceKeyword` in `AceEditorProvider.tsx` copies only `name/value/score/meta/docText/docHTML` and drops `completer`, so `insertMatch` never runs and `addTable` is not called on selection. Ace inserts `value` directly, which is why the fix quotes `value`. No quoted name reaches table state or the metadata API through this path. The `addTable` side effect on select is already inactive for the same reason, independent of this 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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4814436726
@bito-code-review I traced this before merging and I think it's a false positive in our case. The quoted value only reaches addTable via the completer's insertMatch, but the stock SQL Lab editor renders through EditorHost -> AceEditorProvider, whose toAceKeyword mapper drops the completer field entirely, so insertMatch never runs and addTable never sees the quoted string. The quoting only affects the text Ace inserts into the buffer, which is exactly the intended fix. Happy to reopen if there's an editor path I'm missing.
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4769288036
<!-- Bito Reply -->
The flagged issue is correct. The current implementation uses the quoted identifier as the `value` property for autocomplete, which is then passed directly to `addTable` upon selection. This causes the application to store the quoted string as the canonical table name, leading to failures in downstream API calls that expect the raw table name.
To resolve this, you should separate the value used for editor insertion (which requires quoting) from the value used for state/API identification (which should be the raw name). You can update the `completer` object in `useKeywords.ts` to handle the insertion logic separately, ensuring `addTable` receives the raw `value` while the editor still receives the quoted string for correct SQL syntax.
Would you like me to fetch all other comments on this PR to validate and implement fixes for them as well?
**superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts**
```
name: label,
value: value, // Use raw value for state/API
schema: tableSchema,
score: TABLE_AUTOCOMPLETE_SCORE,
meta: 'table',
completer: {
insertMatch: (editor: any, data: any) => {
editor.insert(quoteIdentifier(value)); // Insert quoted value into editor
addTable(..., value, ...); // Use raw value for state
}
}
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4744232840
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
<details>
<summary>
Bito Usage Guide
</summary>
**Commands**
Type the following command in the pull request comment and save the comment.
- `/review` - Manually triggers a full AI review.
- `/pause` - Pauses automatic reviews on this pull request.
- `/resume` - Resumes automatic reviews.
- `/resolve` - Marks all Bito-posted review comments as resolved.
- `/abort` - Cancels all in-progress reviews.
Refer to the <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/available-commands" target="_blank" rel="noopener noreferrer">documentation</a> for additional commands.
**Configuration**
This repository uses `Superset` You can customize the agent settings <a href="https://alpha.bito.ai/home/ai-agents/code-review-agent" target="_blank" rel="noopener noreferrer">here</a> or contact your Bito workspace admin at evan@preset.io.
**Documentation & Help**
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/getting-started/install-run-using-bito-cloud/create-or-customize-an-agent-instance" target="_blank" rel="noopener noreferrer">Customize agent settings</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/implementing-custom-code-review-rules" target="_blank" rel="noopener noreferrer">Review rules</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent" target="_blank" rel="noopener noreferrer">General documentation</a>
- <a href="https://docs.bito.ai/bito-dev-agents/ai-code-review-agent/faqs" target="_blank" rel="noopener noreferrer">FAQ</a>
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4742475870
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/41199?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 64.33%. Comparing base ([`c218dc4`](https://app.codecov.io/gh/apache/superset/commit/c218dc418ba717a243e6e27a84e6ddcc1cd01577?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8c307be`](https://app.codecov.io/gh/apache/superset/commit/8c307be777d8bc9c2f966cdede7603eef51df365?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #41199 +/- ##
=======================================
Coverage 64.33% 64.33%
=======================================
Files 2651 2651
Lines 144766 144771 +5
Branches 33401 33403 +2
=======================================
+ Hits 93131 93136 +5
Misses 49965 49965
Partials 1670 1670
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/41199/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [javascript](https://app.codecov.io/gh/apache/superset/pull/41199/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `68.49% <100.00%> (+<0.01%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/superset/pull/41199?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "michael-s-molina (via GitHub)" <gi...@apache.org>.
michael-s-molina commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4833927282
Hi @jesperct and @rusackas,
I noticed the fix currently assumes ANSI double-quote identifiers, but—as @jesperct mentioned in the PR description—other dialects (e.g., MySQL’s backticks) use different quoting.
I don't view this requirement as a follow-up but as something that might completely change where this fix is implemented. To avoid potential reversions, could we incorporate dialect awareness in a quick follow-up PR?
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #41199:
URL: https://github.com/apache/superset/pull/41199#issuecomment-4742341372
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on code in PR #41199:
URL: https://github.com/apache/superset/pull/41199#discussion_r3452930642
##########
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:
##########
@@ -197,7 +205,7 @@ export function useKeywords(
() =>
allCachedTables.map(({ value, label, schema: tableSchema }) => ({
name: label,
- value,
+ value: quoteIdentifier(value),
Review Comment:
**Suggestion:** Using the quoted identifier as the table completion `value` breaks downstream table metadata flows because this `value` is also passed to `addTable` on selection. That stores names like `"COVID Vaccines"` (with quotes) in pinned table state, and later metadata requests use that quoted string as the `name` query param, which can fail table lookups that expect the raw table name. Keep autocomplete insertion quoting separate from the canonical table identity used by state/API calls. [api mismatch]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ SQL Lab table preview fails for quoted-name tables.
- ❌ Table metadata panel shows missing columns for those tables.
- ⚠️ Column autocomplete misses columns from pinned quoted tables.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open SQL Lab editor, which renders `EditorWrapper` at
`superset-frontend/src/SqlLab/components/EditorWrapper/index.tsx:104-120` and passes
`keywords={keywords}` from `useKeywords(...)` at lines 26-35 and 382.
2. `useKeywords` builds `tableKeywords` in
`superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts:204-210`, setting
each table keyword to `{ name: label, value: quoteIdentifier(value), schema: tableSchema,
meta: 'table', completer: { insertMatch } }`, so a table like `COVID Vaccines` produces
`value: "\"COVID Vaccines\""`.
3. In the editor, trigger autocomplete and select the `COVID Vaccines` table completion;
Ace calls `insertMatch` defined in `useKeywords.ts:165-187`, which for `data.meta ===
'table'` dispatches `addTable(..., data.value, catalog ?? null, data.schema ?? schema ??
'', false)` using the quoted `data.value` (`"COVID Vaccines"`) as the `tableName`.
4. `addTable` in `superset-frontend/src/SqlLab/actions/sqlLab.ts:22-37` stores this quoted
`tableName` as `table.name`, and later `SouthPane` at
`superset-frontend/src/SqlLab/components/SouthPane/index.tsx:82-103` passes
`tableName={name}` into `TablePreview`, which uses `tableName` as the `table` parameter to
`useTableMetadataQuery` (`TablePreview/index.tsx:136-143`), leading `tables.ts:14-19` to
call `/api/v1/database/{dbId}/table_metadata/?name="%22COVID%20Vaccines%22"&schema=...`;
because the backend expects the raw table name (`COVID Vaccines`) without quotes, metadata
lookups and previews for tables pinned via autocomplete can fail.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=55d0463327954489afd99457fce67d1b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=55d0463327954489afd99457fce67d1b&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts
**Line:** 208:208
**Comment:**
*Api Mismatch: Using the quoted identifier as the table completion `value` breaks downstream table metadata flows because this `value` is also passed to `addTable` on selection. That stores names like `"COVID Vaccines"` (with quotes) in pinned table state, and later metadata requests use that quoted string as the `name` query param, which can fail table lookups that expect the raw table name. Keep autocomplete insertion quoting separate from the canonical table identity used by state/API calls.
Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41199&comment_hash=de142fbb321ac4139b0415db482b8968e20f5ad537926ab8f6c11579b1311fca&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41199&comment_hash=de142fbb321ac4139b0415db482b8968e20f5ad537926ab8f6c11579b1311fca&reaction=dislike'>👎</a>
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(sqllab): quote autocomplete table names that need it [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #41199:
URL: https://github.com/apache/superset/pull/41199
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org