Re: [PR] fix(dataset): copy catalog field when duplicating a BigQuery dataset [superset]

Sat, 11 Jul 2026 11:02:03 -0700 


codeant-ai-for-open-source[bot] commented on code in PR #41106:
URL: https://github.com/apache/superset/pull/41106#discussion_r3564751638


##########
superset/commands/dataset/duplicate.py:
##########
@@ -75,43 +75,38 @@ def run(self) -> Model:
                 ),
                 status=404,
             )
-        table = SqlaTable(table_name=table_name, editors=editors)
+        table = SqlaTable()
+        table.override(self._base_model)
+        table.table_name = table_name
+        table.editors = editors
         table.database = database
-        table.schema = self._base_model.schema
-        table.catalog = self._base_model.catalog
-        table.template_params = self._base_model.template_params
-        table.normalize_columns = self._base_model.normalize_columns
-        table.always_filter_main_dttm = 
self._base_model.always_filter_main_dttm
         table.is_sqllab_view = True
-        table.sql = self._base_model.sql.strip().strip(";")
+        if table.sql:
+            table.sql = table.sql.strip().strip(";")
         db.session.add(table)
-        cols = []
-        for config_ in self._base_model.columns:
-            column_name = config_.column_name
-            col = TableColumn(
-                column_name=column_name,
-                verbose_name=config_.verbose_name,
-                expression=config_.expression,
+        table.columns = [
+            TableColumn(
+                column_name=c.column_name,
+                verbose_name=c.verbose_name,
+                expression=c.expression,
                 filterable=True,
                 groupby=True,

Review Comment:
   **Suggestion:** The duplicate currently forces every copied column to be 
both groupable and filterable, which changes dataset behavior when the source 
column had either flag disabled. Copy these flags from the source column 
instead of hardcoding them to `True`. [logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ⚠️ Duplicated datasets change column groupby/filterable capabilities.
   - ⚠️ Charts built on duplicates may expose unintended groupby options.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. Create or identify a virtual dataset whose column has groupby=False or
   filterable=False; the TableColumn flags are stored on the ORM model defined 
in
   superset/connectors/sqla/models.py:26-33.
   
   2. In the Superset UI, use the dataset duplicate action, which calls the API 
endpoint at
   superset/datasets/api.py:22-30 (`POST /api/v1/dataset/<pk>/duplicate`) and 
internally
   instantiates DuplicateDatasetCommand(item).run().
   
   3. DuplicateDatasetCommand.validate() in 
superset/commands/dataset/duplicate.py:112-125
   sets self._base_model to the original SqlaTable, including its columns with 
custom
   groupby/filterable flags.
   
   4. During run(), the code at superset/commands/dataset/duplicate.py:87-99 
recreates
   columns with TableColumn(..., filterable=True, groupby=True, ...), causing 
every copied
   column on the new dataset to ignore the source flags and always be both 
groupable and
   filterable.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=a9dcd9cb7b024c39b56d8c2274257097&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=a9dcd9cb7b024c39b56d8c2274257097&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/commands/dataset/duplicate.py
   **Line:** 92:93
   **Comment:**
        *Logic Error: The duplicate currently forces every copied column to be 
both groupable and filterable, which changes dataset behavior when the source 
column had either flag disabled. Copy these flags from the source column 
instead of hardcoding them to `True`.
   
   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%2F41106&comment_hash=0fa197739a0f9d18381f325c0091f076869fc19278d730065f5ae7c209db0701&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41106&comment_hash=0fa197739a0f9d18381f325c0091f076869fc19278d730065f5ae7c209db0701&reaction=dislike'>👎</a>



##########
superset/commands/dataset/duplicate.py:
##########
@@ -75,43 +75,38 @@ def run(self) -> Model:
                 ),
                 status=404,
             )
-        table = SqlaTable(table_name=table_name, editors=editors)
+        table = SqlaTable()
+        table.override(self._base_model)
+        table.table_name = table_name
+        table.editors = editors
         table.database = database
-        table.schema = self._base_model.schema
-        table.catalog = self._base_model.catalog
-        table.template_params = self._base_model.template_params
-        table.normalize_columns = self._base_model.normalize_columns
-        table.always_filter_main_dttm = 
self._base_model.always_filter_main_dttm
         table.is_sqllab_view = True
-        table.sql = self._base_model.sql.strip().strip(";")
+        if table.sql:
+            table.sql = table.sql.strip().strip(";")
         db.session.add(table)
-        cols = []
-        for config_ in self._base_model.columns:
-            column_name = config_.column_name
-            col = TableColumn(
-                column_name=column_name,
-                verbose_name=config_.verbose_name,
-                expression=config_.expression,
+        table.columns = [
+            TableColumn(
+                column_name=c.column_name,
+                verbose_name=c.verbose_name,
+                expression=c.expression,
                 filterable=True,
                 groupby=True,
-                is_dttm=config_.is_dttm,
-                type=config_.type,
-                description=config_.description,
+                is_dttm=c.is_dttm,
+                type=c.type,
+                description=c.description,
             )
-            cols.append(col)
-        table.columns = cols
-        mets = []
-        for config_ in self._base_model.metrics:
-            metric_name = config_.metric_name
-            met = SqlMetric(
-                metric_name=metric_name,
-                verbose_name=config_.verbose_name,
-                expression=config_.expression,
-                metric_type=config_.metric_type,
-                description=config_.description,
+            for c in self._base_model.columns
+        ]
+        table.metrics = [
+            SqlMetric(
+                metric_name=m.metric_name,
+                verbose_name=m.verbose_name,
+                expression=m.expression,
+                metric_type=m.metric_type,
+                description=m.description,
             )

Review Comment:
   **Suggestion:** The metric duplication path also copies only a subset of 
persisted metric fields, so formatting and metadata attributes are lost on 
clone. Preserve the additional stored metric attributes (such as formatting, 
currency, warning text, and `extra`) to avoid changing chart/query behavior 
after duplication. [incomplete implementation]
   
   <details>
   <summary><b>Severity Level:</b> Critical 🚨</summary>
   
   ```mdx
   - ⚠️ Metric number formatting lost on duplicated datasets.
   - ⚠️ Configured currency metadata dropped from cloned metrics.
   - ⚠️ Warning text and extra metadata removed on metric duplicates.
   ```
   </details>
   <details>
   <summary><b>Steps of Reproduction ✅ </b></summary>
   
   ```mdx
   1. On a virtual dataset, configure a saved metric with non-default fields 
such as
   d3format, currency, warning_text, or extra JSON; these are defined on 
SqlMetric in
   superset/connectors/sqla/models.py:275-286 and listed in export_fields at 
lines 292-302.
   
   2. Use the dataset duplicate feature in the UI, invoking the API handler in
   superset/datasets/api.py:22-30 which constructs 
DuplicateDatasetCommand(item) and calls
   .run().
   
   3. DuplicateDatasetCommand.validate() in 
superset/commands/dataset/duplicate.py:112-125
   assigns self._base_model to the original SqlaTable, including its SqlMetric 
objects with
   full metadata.
   
   4. During run(), metrics are recreated at 
superset/commands/dataset/duplicate.py:100-109
   via SqlMetric(metric_name, verbose_name, expression, metric_type, 
description) only;
   fields like d3format, currency, warning_text, and extra are not copied, so 
the cloned
   dataset’s metrics lose formatting, currency configuration, warning text, and 
additional
   metadata, changing chart display and behavior.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=96bbb5b1fdac49d6b62cdb1fc84800c6&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=96bbb5b1fdac49d6b62cdb1fc84800c6&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/commands/dataset/duplicate.py
   **Line:** 101:107
   **Comment:**
        *Incomplete Implementation: The metric duplication path also copies 
only a subset of persisted metric fields, so formatting and metadata attributes 
are lost on clone. Preserve the additional stored metric attributes (such as 
formatting, currency, warning text, and `extra`) to avoid changing chart/query 
behavior after duplication.
   
   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%2F41106&comment_hash=ddac148408efe94d7e4dd7dd2e42131081a4a96533646804d67af74c140d2c62&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F41106&comment_hash=ddac148408efe94d7e4dd7dd2e42131081a4a96533646804d67af74c140d2c62&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]

Reply via email to