Fix: Ensure unique SQL aliases for chart metrics#1392
Draft
sentry[bot] wants to merge 1 commit into
Draft
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR addresses the
InvalidRequestError: Ambiguous column name 'SUM(program_sequence_number)'that occurred when fetching chart data previews.Problem:
The error was caused by the generated SQL query containing duplicate aggregate expressions with identical aliases (e.g.,
SUM(program_sequence_number)appearing twice). SQLAlchemy's result processing failed when attempting to convert rows into dictionaries due to these ambiguous column names.Root Cause:
The
build_multi_metric_queryfunction incharts_service.pywas iterating through all metrics provided in the payload and adding them to the query builder without any deduplication. If the incomingChartDataPayloadcontained two or moreChartMetricobjects that were identical in terms ofcolumnandaggregation, they would both be translated into the SQLSELECTclause with the same alias.Solution:
Two changes were implemented:
Deduplicate Metrics in
build_multi_metric_query:ddpui/core/charts/charts_service.pyto deduplicate metrics based on their(column, aggregation)pair before adding them to theAggQueryBuilder. Aseen_metricsset now tracks processed metrics, ensuring that each unique metric (by column and aggregation) is added only once. This directly prevents the primary cause of duplicate aliases.Ensure Unique Aliases in
AggQueryBuilder(Defense-in-Depth):add_aggregate_columnmethod inddpui/core/datainsights/query_builder.py. TheAggQueryBuildernow maintains an internal_used_aliasesdictionary. If an alias provided for a new aggregate column is already in use, a numeric suffix (e.g.,_2,_3) is automatically appended to make the alias unique. This provides a robust safeguard against any other potential sources of alias collisions.Fixes DALGO-BACKEND-2FW