You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "anthonyhungnguyen (via GitHub)" <gi...@apache.org> on 2026/07/30 18:17:55 UTC
[PR] fix(sqllab): key overwrite-dataset options by id, not table name [superset]
anthonyhungnguyen opened a new pull request, #42620:
URL: https://github.com/apache/superset/pull/42620
### SUMMARY
The "Overwrite existing" dropdown in SQL Lab's **Save or Overwrite Dataset** modal builds its options with `value: r.table_name`:
```ts
data: response.json.result.map(
(r: { table_name: string; id: number; editors: Subject[] }) => ({
value: r.table_name,
label: r.table_name,
datasetId: r.id,
editors: r.editors,
}),
),
```
Table names are not unique — the same table name can exist in several schemas, and a user can be an editor on datasets for all of them (Airflow metadata tables such as `task_instance` saved from a staging and a prod schema are a common case). When that happens:
- duplicate `value`s collide on one Select key, so the listbox intermittently renders the same row several times (reported as "sometimes I get 8 identical rows")
- the labels are identical, so the datasets are indistinguishable to the user
- the resulting selection is ambiguous, so **Overwrite can target the wrong dataset**
This PR keys the options by the dataset `id`, which is unique, and renders a schema-qualified label so same-named datasets can be told apart:
```ts
value: r.id,
label: r.schema ? `${r.schema}.${r.table_name}` : r.table_name,
```
`schema` is already part of the dataset API's `list_columns`, so no backend change is needed. `filterAutocompleteOption` now matches on the label rather than the value, since the value is no longer a string, and `DatasetOptionAutocomplete.value` is typed `number`. The overwrite path itself is unaffected — it already used `datasetToOverwrite.datasetId`.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: with three editable datasets named `task_instance` in different schemas, the dropdown shows repeated, identical `task_instance` rows and there is no way to tell which one Overwrite will hit.
After: one row per dataset, each labelled `staging.task_instance`, `prod.task_instance`, etc., and Overwrite targets the row that was clicked.
### TESTING INSTRUCTIONS
```bash
cd superset-frontend
npx jest src/SqlLab/components/SaveDatasetModal/SaveDatasetModal.test.tsx
```
20/20 pass. Added a regression test — `distinguishes datasets that share a table name and overwrites the selected one` — that stubs two editable datasets sharing the table name `task_instance` across a `staging` and a `prod` schema, asserts each renders exactly once under its schema-qualified label, and asserts that selecting the `prod` row issues the PUT against that dataset's id. The test fails on `master` (both labels resolve to two rendered nodes) and passes with this change.
The three existing test call sites that looked up `'coolest table 0'` were updated to the new qualified label. `prettier --check` and `oxlint` are clean on the touched files; `tsc --noEmit` reports no new errors.
Manual check: open SQL Lab → run a query → **Save** → **Save or Overwrite Dataset** → **Overwrite existing**, with an account that can edit two datasets sharing a table name in different schemas.
### 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): key overwrite-dataset options by id, not table name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-5145713796
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): key overwrite-dataset options by id, not table name [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 #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r4372080660
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -424,7 +470,14 @@ export const SaveDatasetModal = ({
const filterAutocompleteOption = (
inputValue: string,
option: DatasetOverwriteOption,
- ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+ ) => {
+ const label = option.label.toLowerCase();
+ // Position-independent: a dataset may or may not have a catalog, and a
+ // search skipping a part (`examples.sales`) should still match.
+ return parseQualifiedSearch(inputValue.toLowerCase()).parts.every(part =>
+ label.includes(part),
+ );
Review Comment:
✅ **Customized review instruction saved!**
**Instruction:**
> In SaveDatasetModal dataset overwrite filtering, allow qualified search components to match regardless of position so searches can skip the catalog; do not require components to match positional order.
**Applied to:**
- `superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx`
---
💡 *To manage or update this instruction, visit: [CodeAnt AI Settings](https://app.codeant.ai/org/settings/learnings)*
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -342,9 +385,19 @@ export const SaveDatasetModal = ({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `e6b27e3`.
The loader accepts `page` and `pageSize` and includes them as `page` and `page_size` in the dataset API query.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "anthonyhungnguyen (via GitHub)" <gi...@apache.org>.
anthonyhungnguyen commented on code in PR #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r4372079439
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -195,6 +195,37 @@ const updateDataset = async ({
const UNTITLED = t('Untitled Dataset');
+/**
+ * Datasets are unique by database, catalog, schema and table name, so a label
+ * built from anything less can be ambiguous — e.g. `examples.public.sales`.
+ */
+const qualifiedLabel = (dataset: {
+ database?: { database_name?: string };
+ catalog?: string | null;
+ schema?: string | null;
+ table_name: string;
+}) =>
+ [
+ dataset.database?.database_name,
+ dataset.catalog,
+ dataset.schema,
+ dataset.table_name,
+ ]
+ .filter(Boolean)
Review Comment:
Fixed in e6b27e3: the label keeps the schema slot even when it's empty, so `(catalog=reporting, schema=null)` renders as `analytics.reporting..sales` and `(catalog=null, schema=reporting)` as `analytics.reporting.sales`. A missing catalog is still dropped, so the common case reads as before. Search ignores empty parts, so typing either form still matches. Added a test for that pair.
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -342,9 +385,19 @@ export const SaveDatasetModal = ({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
Review Comment:
Valid, fixed in e6b27e3: the loader now takes AsyncSelect's `page` and `pageSize` and sends them as `page` / `page_size`, so scrolling loads the next page instead of the first one again. Added a test that checks the request.
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -321,12 +352,18 @@ export const SaveDatasetModal = ({
};
const loadDatasetOverwriteOptions = useCallback(async (input = '') => {
+ // Only the table part can be filtered server-side — `database` is a
+ // relationship the list endpoint cannot match on by name. Sending the
+ // whole search as a `table_name` filter matches nothing once the user
+ // types a separator; filterAutocompleteOption narrows the qualifiers.
+ const { tableSearch } = parseQualifiedSearch(input);
+
const queryParams = rison.encode({
filters: [
{
col: 'table_name',
opr: 'ct',
- value: input,
+ value: tableSearch,
Review Comment:
The list endpoint can't filter `database` by name, so the qualifiers can only be matched client-side. With paging fixed (see the thread above), datasets on later pages load as you scroll, so a qualified search no longer stops at the first page.
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -424,7 +470,14 @@ export const SaveDatasetModal = ({
const filterAutocompleteOption = (
inputValue: string,
option: DatasetOverwriteOption,
- ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+ ) => {
+ const label = option.label.toLowerCase();
+ // Position-independent: a dataset may or may not have a catalog, and a
+ // search skipping a part (`examples.sales`) should still match.
+ return parseQualifiedSearch(inputValue.toLowerCase()).parts.every(part =>
+ label.includes(part),
+ );
Review Comment:
Leaving this as is on purpose. Matching parts regardless of position lets a search skip the catalog (`analytics.prod.task_instance` finds `analytics.reporting.prod.task_instance`). Each option shows its full label, so an extra match can't be mistaken for the one being searched.
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "anthonyhungnguyen (via GitHub)" <gi...@apache.org>.
anthonyhungnguyen commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-6488234983
@rusackas Done in e6b27e3: the schema slot is kept when empty, so catalog-only and schema-only datasets render as `db.reporting..sales` vs `db.reporting.sales`, with a test for that pair. Also merged master to fix the stale-branch CI failures.
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "codecov[bot] (via GitHub)" <gi...@apache.org>.
codecov[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-5134766044
## [Codecov](https://app.codecov.io/gh/apache/superset/pull/42620?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `66.66667%` with `1 line` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.45%. Comparing base ([`6e1c2fb`](https://app.codecov.io/gh/apache/superset/commit/6e1c2fbcb8f2301723103be91d53df895930bea9?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`8c10eae`](https://app.codecov.io/gh/apache/superset/commit/8c10eaeb39d6df0f9fbeaabf2ecffa6448fdd88f?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
| [Files with missing lines](https://app.codecov.io/gh/apache/superset/pull/42620?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [...d/src/SqlLab/components/SaveDatasetModal/index.tsx](https://app.codecov.io/gh/apache/superset/pull/42620?src=pr&el=tree&filepath=superset-frontend%2Fsrc%2FSqlLab%2Fcomponents%2FSaveDatasetModal%2Findex.tsx&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1NhdmVEYXRhc2V0TW9kYWwvaW5kZXgudHN4) | 66.66% | [1 Missing :warning: ](https://app.codecov.io/gh/apache/superset/pull/42620?src=pr&el=tree&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 #42620 +/- ##
=======================================
Coverage 65.44% 65.45%
=======================================
Files 2810 2810
Lines 159335 159336 +1
Branches 36362 36363 +1
=======================================
+ Hits 104279 104286 +7
+ Misses 53013 53007 -6
Partials 2043 2043
```
| [Flag](https://app.codecov.io/gh/apache/superset/pull/42620/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/42620/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `71.54% <66.66%> (+<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/42620?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): key overwrite-dataset options by id, not table name [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 #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3692076434
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -321,12 +352,18 @@ export const SaveDatasetModal = ({
};
const loadDatasetOverwriteOptions = useCallback(async (input = '') => {
+ // Only the table part can be filtered server-side — `database` is a
+ // relationship the list endpoint cannot match on by name. Sending the
+ // whole search as a `table_name` filter matches nothing once the user
+ // types a separator; filterAutocompleteOption narrows the qualifiers.
+ const { tableSearch } = parseQualifiedSearch(input);
+
const queryParams = rison.encode({
filters: [
{
col: 'table_name',
opr: 'ct',
- value: input,
+ value: tableSearch,
Review Comment:
**Suggestion:** When the input contains a separator, only the table-name suffix is sent to the API and qualifier filtering is deferred to the browser. The async select receives only the first API page, so if more than one page of editable datasets shares that table-name suffix, a matching database/schema dataset on a later page is never loaded and cannot be selected. Apply the qualifier filters server-side, or request and merge all relevant pages before client-side filtering. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Large table-name result sets hide editable datasets.
- ❌ Users cannot overwrite matching datasets beyond page one.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b65a4ec1b11e4522a532d744857eaf26&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=b65a4ec1b11e4522a532d744857eaf26&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/SaveDatasetModal/index.tsx
**Line:** 366:366
**Comment:**
*Logic Error: When the input contains a separator, only the table-name suffix is sent to the API and qualifier filtering is deferred to the browser. The async select receives only the first API page, so if more than one page of editable datasets shares that table-name suffix, a matching database/schema dataset on a later page is never loaded and cannot be selected. Apply the qualifier filters server-side, or request and merge all relevant pages before client-side filtering.
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%2F42620&comment_hash=d30a27020387323c6d15ea2a456e47691cac18f0c639071ffb2f8148551814af&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42620&comment_hash=d30a27020387323c6d15ea2a456e47691cac18f0c639071ffb2f8148551814af&reaction=dislike'>👎</a>
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -424,7 +470,14 @@ export const SaveDatasetModal = ({
const filterAutocompleteOption = (
inputValue: string,
option: DatasetOverwriteOption,
- ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+ ) => {
+ const label = option.label.toLowerCase();
+ // Position-independent: a dataset may or may not have a catalog, and a
+ // search skipping a part (`examples.sales`) should still match.
+ return parseQualifiedSearch(inputValue.toLowerCase()).parts.every(part =>
+ label.includes(part),
+ );
Review Comment:
**Suggestion:** The client-side filter treats every qualified-search component as an unordered substring, so a search such as `foo.bar` can match a dataset whose label contains `bar.foo` or where one component merely occurs inside another value. This can present unrelated datasets and make the selection ambiguous. Match the qualified components in their actual database/catalog/schema/table order, or use structured fields for filtering. [incorrect condition logic]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Qualified searches can show unrelated datasets.
- ⚠️ SQL Lab overwrite selection remains ambiguous.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=34b9f28052e7401f8087c5ff5db57c4a&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=34b9f28052e7401f8087c5ff5db57c4a&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/SaveDatasetModal/index.tsx
**Line:** 477:479
**Comment:**
*Incorrect Condition Logic: The client-side filter treats every qualified-search component as an unordered substring, so a search such as `foo.bar` can match a dataset whose label contains `bar.foo` or where one component merely occurs inside another value. This can present unrelated datasets and make the selection ambiguous. Match the qualified components in their actual database/catalog/schema/table order, or use structured fields for filtering.
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%2F42620&comment_hash=33d7b75202adc0df77e191f4d2074a20a3eb064be4beac74c33bfeb3e14394c6&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42620&comment_hash=33d7b75202adc0df77e191f4d2074a20a3eb064be4beac74c33bfeb3e14394c6&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): key overwrite-dataset options by id, not table name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-5135794042
<!-- Bito Reply -->
The pull request addresses this issue by changing how dataset options are keyed and filtered in the `SaveDatasetModal`. Previously, options were keyed by `table_name`, which caused collisions when multiple datasets shared the same name. The changes update the `value` to use the unique `id` and update the `label` to include the schema (e.g., `schema.table_name`). Additionally, the `filterAutocompleteOption` function was updated to filter based on the `label` (which now includes the schema) rather than the `value` (the ID), ensuring that typing a schema prefix correctly narrows the list to the intended dataset.
**superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx**
```
// `id` is unique; `table_name` is not. Keying options by the table
// name collapses same-named datasets onto a single Select key, which
// renders duplicate rows and makes the overwrite target ambiguous.
value: r.id,
label: r.schema ? `${r.schema}.${r.table_name}` : r.table_name,
```
**superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx**
```
const filterAutocompleteOption = (
inputValue: string,
option: DatasetOverwriteOption,
) => option.label.toLowerCase().includes(inputValue.toLowerCase());
```
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-5136531170
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): key overwrite-dataset options by id, not table name [superset]
Posted by "netlify[bot] (via GitHub)" <gi...@apache.org>.
netlify[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-5145424501
### <span aria-hidden="true">✅</span> Deploy Preview for *superset-docs-preview* ready!
| Name | Link |
|:-:|------------------------|
|<span aria-hidden="true">🔨</span> Latest commit | 7e013dd78b73b17c82e5fbd6bfbd975698c98471 |
|<span aria-hidden="true">🔍</span> Latest deploy log | https://app.netlify.com/projects/superset-docs-preview/deploys/6a6cd3b6389fc500084df0b1 |
|<span aria-hidden="true">😎</span> Deploy Preview | [https://deploy-preview-42620--superset-docs-preview.netlify.app](https://deploy-preview-42620--superset-docs-preview.netlify.app) |
|<span aria-hidden="true">📱</span> Preview on mobile | <details><summary> Toggle QR Code... </summary><br /><br /><br /><br />_Use your smartphone camera to open QR code link._</details> |
|<span aria-hidden="true">🤖</span> Make changes | [Run an agent on this branch](https://app.netlify.com/projects/superset-docs-preview/agent-runs#DctRCoMwDIDhu-R5Yodb0d1gx0ibFgtqQpvpxvDu5u3jh_8PUnkVhRfADULFLc7mXL59wz11hIotacd7qkctauUjS4loYtHCW7OPkiz8e5OdHn2kIfhhnHJ8OufGB2UX7nBe) |
---
<!-- [superset-docs-preview Preview](https://deploy-preview-42620--superset-docs-preview.netlify.app) -->
_To edit notification comments on pull requests, go to your [Netlify project configuration](https://app.netlify.com/projects/superset-docs-preview/configuration/notifications#deploy-notifications)._
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-6221894033
Thanks @anthonyhungnguyen, keying by id is a good call, and the paging/search fixes look solid. The one open bit is @sadpandajoe's last thread: if empty qualifier slots get dropped, a dataset with only a catalog and one with only a schema can render the same label. Could you keep the positions or label each part?
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3708287658
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -195,6 +195,37 @@ const updateDataset = async ({
const UNTITLED = t('Untitled Dataset');
+/**
+ * Datasets are unique by database, catalog, schema and table name, so a label
+ * built from anything less can be ambiguous — e.g. `examples.public.sales`.
+ */
+const qualifiedLabel = (dataset: {
+ database?: { database_name?: string };
+ catalog?: string | null;
+ schema?: string | null;
+ table_name: string;
+}) =>
+ [
+ dataset.database?.database_name,
+ dataset.catalog,
+ dataset.schema,
+ dataset.table_name,
+ ]
+ .filter(Boolean)
Review Comment:
Dropping empty qualifier slots makes distinct datasets such as `(catalog=reporting, schema=null)` and `(catalog=null, schema=reporting)` render the same label, so the overwrite picker cannot tell users which dataset they are selecting. Could this preserve the qualifier positions or label each component explicitly?
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3685939155
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -424,7 +432,7 @@ export const SaveDatasetModal = ({
const filterAutocompleteOption = (
inputValue: string,
option: DatasetOverwriteOption,
- ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+ ) => option.label.toLowerCase().includes(inputValue.toLowerCase());
Review Comment:
When the editable dataset list spans more than one page, typing the new schema prefix still sends `prod.` to the API as a `table_name` filter, so the async response replaces the options with an empty list even when `prod.task_instance` exists. Could the server query search schema/qualified labels too, or otherwise keep schema filtering client-side?
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "anthonyhungnguyen (via GitHub) via notifications" <no...@superset.apache.org>.
anthonyhungnguyen commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-6787169329
@rusackas The label thread you pointed to is addressed in e6b27e3 (the schema slot is kept when empty, with a test for the catalog-only vs schema-only pair), and I have resolved the review threads. CI is green on the current head. Could you take another look when you have a moment? Happy to merge master again or make further changes.
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "anthonyhungnguyen (via GitHub)" <gi...@apache.org>.
anthonyhungnguyen commented on code in PR #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3686200385
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -424,7 +432,7 @@ export const SaveDatasetModal = ({
const filterAutocompleteOption = (
inputValue: string,
option: DatasetOverwriteOption,
- ) => option.value.toLowerCase().includes(inputValue.toLowerCase());
+ ) => option.label.toLowerCase().includes(inputValue.toLowerCase());
Review Comment:
Good catch — you're right, and the regression test I'd written was masking it: the `SupersetClient.get` stub ignored the request, so the search string never had to match anything server-side.
Fixed in c800f64. The search string is now split on the first dot and sent as two filters, `schema` and `table_name` (both are in the dataset API's `search_columns`), so `prod.task_` reaches the server as `schema ct 'prod'` + `table_name ct 'task_'` instead of a `table_name` filter that can never match. `filterAutocompleteOption` mirrors the same split, so the local pass — which only exists to hide options left over from an earlier search — can't hide a row the API deliberately returned (e.g. `prod.` vs. schema `production`).
The test now pads the fixture past one API page and applies the rison filters the way the API does. I checked it's not vacuous: reverting just the server-side split makes it fail on exactly the scenario you described (the response comes back empty and the `prod.task_instance` row disappears).
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "sadpandajoe (via GitHub)" <gi...@apache.org>.
sadpandajoe commented on code in PR #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3686709336
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -342,9 +371,17 @@ export const SaveDatasetModal = ({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
}).then(response => ({
data: response.json.result.map(
- (r: { table_name: string; id: number; editors: Subject[] }) => ({
- value: r.table_name,
- label: r.table_name,
+ (r: {
+ table_name: string;
+ id: number;
+ editors: Subject[];
+ schema?: string;
+ }) => ({
+ // `id` is unique; `table_name` is not. Keying options by the table
+ // name collapses same-named datasets onto a single Select key, which
+ // renders duplicate rows and makes the overwrite target ambiguous.
+ value: r.id,
+ label: r.schema ? `${r.schema}.${r.table_name}` : r.table_name,
Review Comment:
Datasets are unique by database/catalog/schema/table, so editable datasets from different databases or catalogs can still render the same label here and leave the user unable to tell which one will be overwritten. Could the label and autocomplete search include the database/catalog identity as well?
--
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): key overwrite-dataset options by id, not table name [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 #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3691143048
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -342,9 +385,19 @@ export const SaveDatasetModal = ({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
Review Comment:
**Suggestion:** The loader ignores the `page` and `pageSize` arguments required by `AsyncSelect` and never sends pagination parameters to the dataset API. When more than one page matches the table-name filter, subsequent scroll requests retrieve the same first page, so editable datasets beyond the first page cannot be selected. Accept the pagination arguments and include the corresponding page and page-size query parameters. [api mismatch]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Large overwrite lists cannot expose later datasets.
- ❌ SQL Lab users cannot select some editable datasets.
- ⚠️ AsyncSelect pagination repeatedly fetches page one.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=ff1c4c69668e49698f9d07e55d35ce6d&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=ff1c4c69668e49698f9d07e55d35ce6d&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/SaveDatasetModal/index.tsx
**Line:** 367:385
**Comment:**
*Api Mismatch: The loader ignores the `page` and `pageSize` arguments required by `AsyncSelect` and never sends pagination parameters to the dataset API. When more than one page matches the table-name filter, subsequent scroll requests retrieve the same first page, so editable datasets beyond the first page cannot be selected. Accept the pagination arguments and include the corresponding page and page-size query parameters.
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%2F42620&comment_hash=d7082df8766dff70aba5be331346fc4ea9aee5b5fcdd0baae22c4274cd1a7829&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F42620&comment_hash=d7082df8766dff70aba5be331346fc4ea9aee5b5fcdd0baae22c4274cd1a7829&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): key overwrite-dataset options by id, not table name [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-5134643520
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): key overwrite-dataset options by id, not table name [superset]
Posted by "anthonyhungnguyen (via GitHub)" <gi...@apache.org>.
anthonyhungnguyen commented on code in PR #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r3691111403
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -342,9 +371,17 @@ export const SaveDatasetModal = ({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
}).then(response => ({
data: response.json.result.map(
- (r: { table_name: string; id: number; editors: Subject[] }) => ({
- value: r.table_name,
- label: r.table_name,
+ (r: {
+ table_name: string;
+ id: number;
+ editors: Subject[];
+ schema?: string;
+ }) => ({
+ // `id` is unique; `table_name` is not. Keying options by the table
+ // name collapses same-named datasets onto a single Select key, which
+ // renders duplicate rows and makes the overwrite target ambiguous.
+ value: r.id,
+ label: r.schema ? `${r.schema}.${r.table_name}` : r.table_name,
Review Comment:
Agreed — fixed in 9ddff12.
Options are now labelled with every part that is set: `database.catalog.schema.table_name` (e.g. `analytics.reporting.prod.task_instance`), which is the actual uniqueness key.
On the search: only the table part can go to the API. The qualifiers span three columns, and `database` is a relationship the list endpoint can't match on by name (`search_columns` exposes it, but not as a `ct` filter on `database_name`). So the request sends the trailing part of the search as the `table_name` filter and the qualifiers are narrowed client-side over the rows that came back. Parts are matched positionally-independently, so `analytics.prod.sales` still matches a dataset that also has a catalog, and a trailing `.` is treated as qualification rather than a table name.
That does mean the qualifier narrowing only sees the rows the table search returned — a limitation if more than a page of datasets share one table name, though that's far rarer than having more than a page of datasets overall, which was the case that broke before. This also replaces the server-side `schema` filter from the previous commit: it can't be kept, since a two-part search can just as easily be `database.table` as `schema.table`, and guessing wrong sends a filter that matches nothing.
Test covers three datasets sharing `task_instance` across two databases and two schemas, one with a catalog, with the fixture padded past one API page. I checked it's not vacuous — it fails if the database is dropped from the label, if the whole search string is sent as the `table_name` filter, or if the local filter is stubbed out.
--
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): key overwrite-dataset options by id, not table name [superset]
Posted by "codeant-ai-for-open-source[bot] (via GitHub)" <gi...@apache.org>.
codeant-ai-for-open-source[bot] commented on PR #42620:
URL: https://github.com/apache/superset/pull/42620#issuecomment-6402048487
## <a href="https://codeant.ai/" title="CodeAnt AI"><img src="https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/codeant-block-purple-v2.svg" width="28" height="28" align="absmiddle" style="vertical-align:middle" alt="CodeAnt" /></a> CodeAnt PR Risk
**🟡 Medium Risk**
- The PR needs attention before merging because qualified searches can hide matching datasets beyond the first API page until users load more results.
**Assessed commit:** `e6b27e3c64fd`
--
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): key overwrite-dataset options by id, not table name [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 #42620:
URL: https://github.com/apache/superset/pull/42620#discussion_r4408770379
##########
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx:
##########
@@ -342,9 +385,19 @@ export const SaveDatasetModal = ({
endpoint: `/api/v1/dataset/?q=${queryParams}`,
Review Comment:
✅ **CodeAnt verified this suggestion was addressed in subsequent commits and marked this thread resolved** as of `d682936`.
The loader now accepts `page` and `pageSize` and includes them as `page` and `page_size` in the dataset API query.
<sub>If that's not right, unresolve this thread and CodeAnt will leave it open.</sub>
<!-- codeant-auto-resolve-reply -->
--
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