Skip to content

Add dunder prefix to column aliases for simple-metric inputs#1938

Merged
plypaul merged 6 commits intomainfrom
p/alias_simple_metric__03
Nov 17, 2025
Merged

Add dunder prefix to column aliases for simple-metric inputs#1938
plypaul merged 6 commits intomainfrom
p/alias_simple_metric__03

Conversation

@plypaul
Copy link
Contributor

@plypaul plypaul commented Nov 13, 2025

To avoid alias conflicts in the generated SQL when a semantic model has a dimension and a simple metric with the same name, this PR updates the naming scheme for SQL column aliases. The longer term fix is to add a validation so that semantic models are required to have unique metric / measure / entity names.

Due to the large snapshot changes, please view by commit.

@cla-bot cla-bot bot added the cla:yes label Nov 13, 2025
@plypaul plypaul marked this pull request as ready for review November 13, 2025 07:45
@plypaul plypaul requested a review from a team as a code owner November 13, 2025 07:45
Copy link
Contributor

@courtneyholcomb courtneyholcomb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was a lot simpler than I expected! Thank you!

@plypaul plypaul force-pushed the p/alias_simple_metric__02 branch from 0f9bb9c to 6631e01 Compare November 17, 2025 16:54
@plypaul plypaul force-pushed the p/alias_simple_metric__03 branch from 80fd912 to f341cb8 Compare November 17, 2025 16:54
plypaul added a commit that referenced this pull request Nov 17, 2025
Prior to the `measure -> simple metric` migration, aliases could be
specified for measures (e.g. for input measures in a ratio metric) and
for metrics (e.g. for input metrics in a derived metric).

Post-migration, the code retained some of the prior behavior by
replacing measures with a similar data structure called a simple-metric
input. Aggregation of the simple-metric inputs was handled by
`AggregateSimpleMetricInputsNode`, and it also handled renaming columns
to handle manifest-configured aliases for simple metrics. The
`ComputeMetricsNode` handled renaming columns for metric aliases.

Since inputs to all metrics (aside from simple metrics) are other
metrics now, renaming columns for aliases can be consolidated to the
`ComputeMetricsNode`. This helps resolves some issues related to
#1938.

Due to the large snapshot changes, please view by commit.
@plypaul plypaul force-pushed the p/alias_simple_metric__02 branch from 6631e01 to 79e52f8 Compare November 17, 2025 17:05
@plypaul plypaul force-pushed the p/alias_simple_metric__03 branch from f341cb8 to e04aa50 Compare November 17, 2025 17:05
Base automatically changed from p/alias_simple_metric__02 to main November 17, 2025 17:15
@plypaul plypaul force-pushed the p/alias_simple_metric__03 branch from e04aa50 to 66bfc2c Compare November 17, 2025 17:16
@plypaul plypaul merged commit c9c54c8 into main Nov 17, 2025
13 checks passed
@plypaul plypaul deleted the p/alias_simple_metric__03 branch November 17, 2025 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants