Preserve role qualifying cube column identity - #2370
Conversation
✅ Deploy Preview for thriving-cassata-78ae72 canceled.
|
|
|
||
| def to_spec(self): | ||
| @property | ||
| def cube_element_name(self) -> str: |
There was a problem hiding this comment.
Could this be named semantic_name so that it's aligned with other usage like
dj/datajunction-server/datajunction_server/models/query.py
Lines 77 to 80 in 05521c0
There was a problem hiding this comment.
i called it cube_element_name to avoid misuse. Column is pretty overloaded since it can represent columns across the various node types. the dimension_column field behaves differently depending on the node type this column is associated with. For exmaple, it carries the referenced dimension name for a dimension node column:
dimension node column: orders.country_code
dimension_id -> common.country
dimension_column -> country_code
whereas for a cube column, dimension_column carries the bracketed role eg [ship_date]. So if the column object represents a column on a dimension node, this function would produce country_codecountry_code.
i do agree that it would be nice to have a more robust and canonical way of resolving column identifiers, but that change would have a wider blast radius. that's what i was alluding to in #2371
| ] | ||
| return ( | ||
| f"{upsert.job.name.lower()}__{upsert.strategy.name.lower()}" # type: ignore | ||
| + (f"__{partition_names[0]}" if temporal_partitions else "") |
There was a problem hiding this comment.
Does this assume at most one temporal partition? With two, partition_names[0] uses only the first and skips the second, so it vanishes from the identity.
| return None # pragma: no cover | ||
|
|
||
| def categorical_expression(self): | ||
| def categorical_expression(self, column_name: Optional[str] = None): |
There was a problem hiding this comment.
Could column_name not be passed in if this just uses self.column.cube_element_name instead?
| """ | ||
| return self.name + (self.dimension_column or "") | ||
|
|
||
| def to_spec(self, *, include_cube_role: bool = False): |
There was a problem hiding this comment.
Can the flag include_cube_role be removed by doing something like:
def to_spec(self):
role = None
if self.node_revision.type == NodeType.CUBE and self.dimension_column:
role = self.dimension_column.strip("[]")
return ColumnSpec(name=self.name, role=role, ...)This is roughly similar to what DimensionLinkSpec does for example
| ) | ||
|
|
||
|
|
||
| def _materialization_name( |
There was a problem hiding this comment.
Since the name is a persisted identity, this format change could orphan existing materializations right? I guess it should just be cases where cubes are referencing dimensions with roles, which wouldn't have been properly supported anyway. Worth doing a migration check though just in case.
| "query": getattr(node, "query", None), | ||
| "columns": [ | ||
| {"name": col.name, "type": col.type} for col in node.columns | ||
| { |
There was a problem hiding this comment.
It looks like this is just in the --format json branch and the default text branch doesn't get the change to display cube cols with roles.
| self.order = order | ||
|
|
||
| @property | ||
| def cube_element_name(self) -> str: |
There was a problem hiding this comment.
Similar comment to below -- could this be semantic_name instead?
2 participants
Summary
Cubes can use the same business dimension in multiple roles. For example, one cube may contain both an order date and a ship date, even though both come from the same underlying date table.
Some DataJunction code paths currently identify these columns only as “date,” losing the role that distinguishes them. This can cause settings intended for one role to be applied to another, columns to disappear from output, or materializations to use the wrong partition. Because these failures can occur silently, they create a data-correctness and operational risk.
This pr hardens the full cube lifecycle so each role remains distinct across:
Expected outcome
An order-date column and a ship-date column will remain independently identifiable and configurable throughout DJ. Ambiguous requests will fail with a clear error instead of silently selecting a column.
Success criteria
Automated tests cover creation, updates, deployment, materialization, and user-facing workflows.