-
Notifications
You must be signed in to change notification settings - Fork 32
feat: semi-additive measures #2502
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
7bb3756
19bcbd6
2dd882e
6b37ea6
3a8c202
9cae7a6
a2fd915
161e255
ab7c6d8
5beb5b4
f0e41cf
d94c413
8764f1b
f043851
5b3ba3f
01c2103
9ab6f79
17e0ca9
5b698bc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| """ | ||
| Add reaggregate column to noderevision | ||
|
|
||
| Revision ID: rg0001reaggregate | ||
| Revises: cm0003dropowner | ||
| Create Date: 2026-08-24 00:00:00.000000+00:00 | ||
| """ | ||
|
|
||
| import sqlalchemy as sa | ||
| from alembic import op | ||
|
|
||
| # revision identifiers, used by Alembic. | ||
| revision = "rg0001reaggregate" | ||
| down_revision = "cm0003dropowner" | ||
| branch_labels = None | ||
| depends_on = None | ||
|
|
||
|
|
||
| def upgrade(): | ||
| op.add_column( | ||
| "noderevision", | ||
| sa.Column("reaggregate", sa.JSON(), nullable=True), | ||
| ) | ||
|
|
||
|
|
||
| def downgrade(): | ||
| op.drop_column("noderevision", "reaggregate") |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,6 +21,7 @@ | |
| ) | ||
| from datajunction_server.construction.build_v3.decomposition import ( | ||
| decompose_and_group_metrics, | ||
| missing_reaggregate_dimensions, | ||
| ) | ||
| from datajunction_server.construction.build_v3.dimensions import parse_dimension_ref | ||
| from datajunction_server.construction.build_v3.filters import ( | ||
|
|
@@ -320,6 +321,14 @@ async def setup_build_context( | |
|
|
||
| # Add dimensions referenced in metric expressions (e.g., LAG ORDER BY) | ||
| add_dimensions_from_metric_expressions(ctx, ctx.decomposed_metrics) | ||
| output_dimensions_after_expression_scan = list(ctx.dimensions) | ||
| internal_reaggregate_dimensions = missing_reaggregate_dimensions( | ||
| ctx.decomposed_metrics.values(), | ||
| output_dimensions_after_expression_scan, | ||
| ) | ||
| for dimension in internal_reaggregate_dimensions: | ||
| if dimension not in ctx.dimensions: | ||
| ctx.dimensions.append(dimension) | ||
|
|
||
| # A second load_nodes pass is needed when either: | ||
| # 1. metric expressions introduced dimension nodes not yet in ctx.nodes, OR | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this is just a parent-column protected dimension (e.g., it's not the fully qualified node name like POST /nodes/metric/
{"query": "SELECT SUM(line_total) FROM v3.order_details",
"reaggregate": {"rules": [{"dimension": "order_date", "fn": "last_value"}]}}
-> 201, status: valid
GET /sql/metrics/v3/?metrics=v3.balance&dimensions=v3.product.category
-> 422 "Reference `order_date` is not fully qualified. Use the `node.column` form..."Should this just reject a non-fully-qualified name (and I think the UI might need to change based on that as well)? |
||
|
|
@@ -333,8 +342,15 @@ async def setup_build_context( | |
| } | ||
| missing_dim_nodes = dim_roots_after - ctx.nodes.keys() | ||
| internally_added_roots = dim_roots_after - dim_roots_before_load | ||
| if missing_dim_nodes or internally_added_roots: | ||
| await load_nodes(ctx) | ||
| try: | ||
| if ( | ||
| missing_dim_nodes | ||
| or internally_added_roots | ||
| or internal_reaggregate_dimensions | ||
| ): | ||
| await load_nodes(ctx) | ||
| finally: | ||
| ctx.dimensions = output_dimensions_after_expression_scan | ||
|
|
||
| # Classify filters into dimension filters (WHERE) and metric filters (HAVING) | ||
| # This MUST happen AFTER all nodes are loaded so we can correctly identify | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this part first call the pre-check
cube_matcher._metric_graph_has_reaggregate(since it's already used in other APIs like infind_matching_cube)?