Bug description

Go to Datasets, and click on any virtual dataset.

Click Edit dataset.

Update the SQL query under the Source tab (e.g., change a limit or add a new column).

Switch to the Metrics tab and add a new custom metric (e.g., COUNT(*) + 1).

Click Save.

Actual result: An error appears: "Error saving dataset: One or more metrics already exist"

Screenshots/recordings

Image

Superset version

master / latest-dev

Python version

3.9

Node version

16

Browser

Chrome

Additional context

When editing the dataset, if we edit both the SQL and the metric, we would see this error, can fix it by fetching the latest metrics info/ id

Superset’s onConfirmSave() logic first sends a PUT request to save the dataset changes, including any new metrics and SQL updates. If the SQL was changed, it automatically triggers a second PUT after syncing columns. However, this second request reuses the in-memory currentDatasource.metrics, which doesn’t yet include the freshly assigned metric ids returned by the backend after the first save. As a result, those metrics still appear to be new (because they lack ids), so the backend treats them as duplicates when it sees the same metric_names being submitted again. This triggers a validation error, with the backend assuming you’re trying to create metrics that already exist.

Per discussion, the easiest fix is to fetch the metrics' id before update.

Checklist

  • [x] I have searched Superset docs and Slack and didn't find a solution to my problem.
  • [x] I have searched the GitHub issue tracker and didn't find a similar bug report.
  • [x] I have checked Superset's logs for errors and if I found a relevant Python stacktrace, I included it here as text in the "additional context" section.

Comment From: dosubot[bot]

Hello @jqqin! I'm here to help you with any bugs, questions, or contributions you have in mind. While you're waiting for a human maintainer, feel free to share more details, and let's tackle this together!

To resolve the metric duplication error when editing a dataset SQL query and adding a new metric simultaneously in Superset, you need to ensure that the metric IDs are correctly fetched and updated before the second PUT request. Here's a suggested approach:

  1. Fetch Latest Metrics Information: After the first PUT request, ensure that the latest metrics information, including their IDs, is fetched from the backend. This will prevent the metrics from being treated as new in the subsequent request.

  2. Update In-Memory Metrics: Before the second PUT request is triggered, update the in-memory currentDatasource.metrics with the freshly assigned metric IDs returned by the backend. This ensures that the metrics are recognized as existing rather than new.

  3. Modify the onConfirmSave Logic: Adjust the onConfirmSave function to include a step that fetches and updates the metric IDs after the first save, before proceeding with any further updates.

By implementing these steps, you should be able to prevent the duplication error from occurring [1].

To continue talking to Dosu, mention @dosu.

Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Bug Report | Other

Comment From: rusackas

@sadpandajoe do our test cases cover this?

Comment From: sadpandajoe

@sadpandajoe do our test cases cover this?

@rusackas we do and I have actually bugged this in our backlog.

Comment From: jqqin

Hi @rusackas @sadpandajoe,thanks a lot for the quick response! 🙏 I’ve encountered this issue in our fork and have implemented a fix that addresses the metric duplication error when editing both the SQL and metrics simultaneously, by fetching the metrics' id before the second PUT request. If it's helpful, I’d be happy to open a PR upstream to contribute the fix. Please let me know your thoughts!

Comment From: sadpandajoe

@jqqin sure, feel free to open up a new PR to address this issue.

Comment From: jqqin

@sadpandajoe @rusackas PR has been created: https://github.com/apache/superset/pull/33523 :)