codeant-ai-for-open-source[bot] commented on code in PR #45848:
URL: https://github.com/apache/superset/pull/45848#discussion_r4413257875
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx:
##########
@@ -1428,28 +1489,50 @@ function DatasourceEditor({
// column the mapped one, and a transform stranded there by an earlier
// writer would otherwise arrive live from a single select. Clearing the
// partition column leaves no mapped column at all, so no column may hold
- // a transform -- which is what the server does too.
+ // a transform -- which is what the server does too. When that costs the
+ // transform the mapping is mirroring, the owner confirms it first.
//
// A transform on a column that is mapped on both sides is untouched, so
// re-pointing the partition column does not cost the owner what they
- // wrote.
- setDatabaseColumns(prev =>
- clearUnmappedTransformsAcrossMove(datasource, next, prev),
- );
- setCalculatedColumns(prev =>
- clearUnmappedTransformsAcrossMove(datasource, next, prev),
+ // wrote, and does not ask.
+ confirmMappingDiscard(
+ [
+ ...clearUnmappedTransformsAcrossMove(
+ datasource,
+ next,
+ databaseColumns,
+ ),
+ ...clearUnmappedTransformsAcrossMove(
+ datasource,
+ next,
+ calculatedColumns,
+ ),
+ ],
+ () => {
+ setDatasource(prev => ({
+ ...prev,
+ partition_column: next.partition_column,
+ partition_mapped_column: next.partition_mapped_column,
+ }));
Review Comment:
**Suggestion:** If metadata sync repairs the mapping while this dialog is
open, accepting applies the captured `next` values and can restore a partition
column that sync removed.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Rarely` ยท ๐ท๏ธ `Race condition`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a30501ad969c433084913ca0ba8835f8&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=a30501ad969c433084913ca0ba8835f8&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.tsx
**Line:** 1512:1516
**Comment:**
*Race Condition: If metadata sync repairs the mapping while this dialog
is open, accepting applies the captured `next` values and can restore a
partition column that sync removed.
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%2F45848&comment_hash=c8d1591b8c5c7b886fd3760ed5cbcae686bb2874eedf446d4b1377936b30da86&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45848&comment_hash=c8d1591b8c5c7b886fd3760ed5cbcae686bb2874eedf446d4b1377936b30da86&reaction=dislike'>๐</a>
##########
superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/PartitionFilterMapping.test.tsx:
##########
@@ -1455,3 +1473,34 @@ test('the unsaved notice is not reported as a failed
verdict', async () => {
).toBeInTheDocument();
expect(onPreviewVerdict).not.toHaveBeenCalledWith(false);
});
+
+test('a stored pass about another partition column is not a confirmation', ()
=> {
+ // The stored verdict is keyed on the transform text alone, so after the
+ // partition column is re-pointed it still reads as a pass -- for a mirror
+ // onto a column nobody has checked it against.
+ render(
+ <PartitionColumnFields
+ datasource={{
+ main_dttm_col: 'event_time',
+ partition_column: 'dt_epoch',
+ partition_filter_mapping: {
+ evaluable: true,
+ evaluated_transform: 'unix_timestamp(:value)',
+ mapped_column: 'event_time',
+ partition_column: 'dt_other',
Review Comment:
**Suggestion:** The dataset API reports these columns from the current
mapping, not the mapping that produced the cached verdict, so this synthetic
stale-pass case misses the real regression.
**Assessment:** ๐ `Major` ยท ๐ `Occurrence: Sometimes` ยท ๐ท๏ธ `Api mismatch`
[](https://docs.codeant.ai/cli/resolve-pr-comments-skill)
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=57ebb5b2324a43018044366ee670cbe0&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=57ebb5b2324a43018044366ee670cbe0&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
<details>
<summary><b>Prompt for AI Agent ๐ค </b></summary>
```mdx
This is a comment left during a code review.
**Path:**
superset-frontend/src/components/Datasource/components/DatasourceEditor/components/PartitionFilterMapping/PartitionFilterMapping.test.tsx
**Line:** 1489:1490
**Comment:**
*Api Mismatch: The dataset API reports these columns from the current
mapping, not the mapping that produced the cached verdict, so this synthetic
stale-pass case misses the real regression.
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%2F45848&comment_hash=8ac1675732d84a76530f5cb5856fedd75e416f097d14235669d782784ba5b9c8&reaction=like'>๐</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F45848&comment_hash=8ac1675732d84a76530f5cb5856fedd75e416f097d14235669d782784ba5b9c8&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: [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]
