From 91f7421f81f8088bd47a829ec03f1e46a77af482 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 18:29:15 -0800 Subject: [PATCH 01/14] Fold dimension attribute properties under a single field for ease of access --- datajunction-server/datajunction_server/models/node.py | 3 +-- datajunction-server/datajunction_server/sql/dag.py | 9 +-------- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/datajunction-server/datajunction_server/models/node.py b/datajunction-server/datajunction_server/models/node.py index 326493e98..657b9e056 100644 --- a/datajunction-server/datajunction_server/models/node.py +++ b/datajunction-server/datajunction_server/models/node.py @@ -575,8 +575,7 @@ class DimensionAttributeOutput(BaseModel): name: str node_name: Optional[str] node_display_name: Optional[str] - is_primary_key: bool - is_hidden: bool + properties: list[str] | None type: str path: List[str] filter_only: bool = False diff --git a/datajunction-server/datajunction_server/sql/dag.py b/datajunction-server/datajunction_server/sql/dag.py index 6ac2964c0..92199f1cc 100644 --- a/datajunction-server/datajunction_server/sql/dag.py +++ b/datajunction-server/datajunction_server/sql/dag.py @@ -483,14 +483,7 @@ def _extract_roles_from_path(join_path) -> str: name=f"{node_name}.{column_name}{_extract_roles_from_path(join_path)}", node_name=node_name, node_display_name=node_display_name, - is_primary_key=( - attribute_types is not None - and ColumnAttributes.PRIMARY_KEY.value in attribute_types - ), - is_hidden=( - attribute_types is not None - and ColumnAttributes.HIDDEN.value in attribute_types - ), + properties=attribute_types.split(",") if attribute_types else [], type=str(column_type), path=[ (path.replace("[", "").replace("]", "")[:-1]) From d43b441bc1817f21c6aa5fa6c0ad9ee4b57370a5 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 18:30:18 -0800 Subject: [PATCH 02/14] Add list of column attribute names for the database object --- datajunction-server/datajunction_server/database/column.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/datajunction-server/datajunction_server/database/column.py b/datajunction-server/datajunction_server/database/column.py index f1d7e0c76..804f594fd 100644 --- a/datajunction-server/datajunction_server/database/column.py +++ b/datajunction-server/datajunction_server/database/column.py @@ -117,6 +117,12 @@ def has_attribute(self, attribute_name: str) -> bool: for attr in self.attributes # pylint: disable=not-an-iterable ) + def attribute_names(self) -> list[str]: + """ + A list of column attribute names + """ + return [attr.attribute_type.name for attr in self.attributes] + def has_attributes_besides(self, attribute_name: str) -> bool: """ Whether the column has any attribute besides the one specified. From 0f446bca769284d5b9209acbdab009fa250ddd8c Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 18:30:37 -0800 Subject: [PATCH 03/14] Add graphql query for retrieving common dimensions --- .../datajunction_server/api/graphql/main.py | 34 ++++++++++++++++- .../api/graphql/scalars/node.py | 37 +++++++++++++++++-- 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/datajunction-server/datajunction_server/api/graphql/main.py b/datajunction-server/datajunction_server/api/graphql/main.py index eb8e49112..6fe6eacac 100644 --- a/datajunction-server/datajunction_server/api/graphql/main.py +++ b/datajunction-server/datajunction_server/api/graphql/main.py @@ -10,9 +10,10 @@ from datajunction_server.api.graphql.engines import EngineInfo, list_engines from datajunction_server.api.graphql.resolvers.nodes import find_nodes_by from datajunction_server.api.graphql.scalars import Connection -from datajunction_server.api.graphql.scalars.node import Node +from datajunction_server.api.graphql.scalars.node import DimensionAttribute, Node from datajunction_server.models.node import NodeCursor, NodeType -from datajunction_server.utils import get_session, get_settings +from datajunction_server.sql.dag import get_shared_dimensions +from datajunction_server.utils import SEPARATOR, get_session, get_settings async def get_context( @@ -151,6 +152,35 @@ async def find_nodes_paginated( ), ) + @strawberry.field( + description="Get common dimensions for the list of metrics", + ) + async def common_dimensions( + self, + names: Annotated[ + Optional[List[str]], + strawberry.argument( + description="A list of metrics to find common dimensions for", + ), + ] = None, + *, + info: Info, + ) -> list[DimensionAttribute]: + """ + Return a list of common dimensions for a set of metrics. + """ + nodes = await find_nodes_by(info, names) + dimensions = await get_shared_dimensions(info.context["session"], nodes) + return [ + DimensionAttribute( # type: ignore + name=dim.name, + attribute=dim.name.split(SEPARATOR)[-1], + properties=dim.attributes, + type=dim.type, + ) + for dim in dimensions + ] + schema = strawberry.Schema(query=Query) diff --git a/datajunction-server/datajunction_server/api/graphql/scalars/node.py b/datajunction-server/datajunction_server/api/graphql/scalars/node.py index 32701217e..a2155fe95 100644 --- a/datajunction-server/datajunction_server/api/graphql/scalars/node.py +++ b/datajunction-server/datajunction_server/api/graphql/scalars/node.py @@ -4,6 +4,7 @@ import strawberry from strawberry.scalars import JSON +from strawberry.types import Info from datajunction_server.api.graphql.scalars import BigInt from datajunction_server.api.graphql.scalars.availabilitystate import AvailabilityState @@ -68,9 +69,35 @@ class DimensionAttribute: # pylint: disable=too-few-public-methods """ name: str - attribute: str - role: str - dimension_node: "NodeRevision" + attribute: str | None + role: str | None = None + properties: list[str] + type: str + + _dimension_node: Optional["Node"] = None + + @strawberry.field(description="The dimension node this attribute belongs to") + async def dimension_node(self, info: Info) -> "Node": + """ + Lazy load the dimension node when queried. + """ + if self._dimension_node: + return self._dimension_node + + dimension_node_name = self.name.rsplit(".", 1)[0] + + # Check if only the 'name' field is requested for dimension_node. Then we can avoid + # the DB call and return a minimal object + requested_fields = { + field.name.value for field in info.field_nodes[0].selection_set.selections + } + if "name" in requested_fields and len(requested_fields) == 1: + return NodeName(name=dimension_node_name) # type: ignore + + return await DBNode.get_by_name( # type: ignore + info.context["session"], + self.name.rsplit(".", 1)[0], + ) @strawberry.type @@ -174,7 +201,9 @@ def cube_dimensions(self, root: "DBNodeRevision") -> List[DimensionAttribute]: ), attribute=element.name, role=dimension_to_roles.get(element.name, ""), - dimension_node=node_revision, + _dimension_node=node_revision, + type=element.type, + properties=element.attribute_names(), ) for element, node_revision in root.cube_elements_with_nodes() if node_revision and node_revision.type != NodeType.METRIC From a03b2d48d8bc3a67471a82d16d8341916d0a2b87 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 21:37:27 -0800 Subject: [PATCH 04/14] Fix client tests --- datajunction-clients/python/tests/test_builder.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/datajunction-clients/python/tests/test_builder.py b/datajunction-clients/python/tests/test_builder.py index 607adc247..5b65e0e65 100644 --- a/datajunction-clients/python/tests/test_builder.py +++ b/datajunction-clients/python/tests/test_builder.py @@ -883,13 +883,12 @@ def test_get_dimensions(self, client): "type": "string", "node_name": "foo.bar.dispatcher", "node_display_name": "Dispatcher", - "is_primary_key": False, + "properties": [], "path": [ "foo.bar.repair_order_details", "foo.bar.repair_order", ], "filter_only": False, - "is_hidden": False, } in result def test_create_namespace(self, client): From 4e6cb99508ca23317d1f86709b3e3f2cb2a0f2b0 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 21:38:37 -0800 Subject: [PATCH 05/14] Fix tests for dimension attribute properties change --- .../datajunction_server/sql/dag.py | 14 +- .../tests/api/dimension_links_test.py | 3 +- datajunction-server/tests/api/metrics_test.py | 299 +++++++----------- datajunction-server/tests/api/nodes_test.py | 104 +++--- datajunction-server/tests/api/sql_test.py | 6 +- datajunction-server/tests/sql/dag_test.py | 6 +- 6 files changed, 160 insertions(+), 272 deletions(-) diff --git a/datajunction-server/datajunction_server/sql/dag.py b/datajunction-server/datajunction_server/sql/dag.py index 92199f1cc..7e18c2a02 100644 --- a/datajunction-server/datajunction_server/sql/dag.py +++ b/datajunction-server/datajunction_server/sql/dag.py @@ -573,20 +573,12 @@ async def get_filter_only_dimensions( name=dim, node_name=link.dimension.name, node_display_name=link.dimension.current.display_name, - is_primary_key=( - dim.split(SEPARATOR)[-1] - in { - col.name for col in link.dimension.current.primary_key() - } - ), type=str(column_mapping[dim.split(SEPARATOR)[-1]].type), path=[upstream.name], filter_only=True, - is_hidden=( - column_mapping[dim.split(SEPARATOR)[-1]].has_attribute( - ColumnAttributes.HIDDEN.value, - ) - ), + properties=column_mapping[ + dim.split(SEPARATOR)[-1] + ].attribute_names(), ) for dim in link.foreign_keys.values() ], diff --git a/datajunction-server/tests/api/dimension_links_test.py b/datajunction-server/tests/api/dimension_links_test.py index fe1c41e7e..966e8dcb4 100644 --- a/datajunction-server/tests/api/dimension_links_test.py +++ b/datajunction-server/tests/api/dimension_links_test.py @@ -859,7 +859,6 @@ async def test_measures_sql_with_reference_dimension_links( assert dimensions_data == [ { "filter_only": False, - "is_primary_key": False, "name": "default.users.registration_country", "node_display_name": "Users", "node_name": "default.users", @@ -867,7 +866,7 @@ async def test_measures_sql_with_reference_dimension_links( "default.events.user_registration_country", ], "type": "string", - "is_hidden": False, + "properties": [], }, ] diff --git a/datajunction-server/tests/api/metrics_test.py b/datajunction-server/tests/api/metrics_test.py index 8758ffb95..15091e604 100644 --- a/datajunction-server/tests/api/metrics_test.py +++ b/datajunction-server/tests/api/metrics_test.py @@ -17,404 +17,364 @@ expected_dimensions = [ { - "filter_only": False, "name": "default.dispatcher.company_name", "node_name": "default.dispatcher", "node_display_name": "Dispatcher", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.dispatcher.dispatcher_id", "node_name": "default.dispatcher", "node_display_name": "Dispatcher", - "is_primary_key": True, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": ["primary_key"], }, { - "filter_only": False, "name": "default.dispatcher.phone", "node_name": "default.dispatcher", "node_display_name": "Dispatcher", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.address", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.birth_date", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "timestamp", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.city", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.contractor_id", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.country", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.first_name", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.hard_hat_id", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": True, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": ["primary_key"], }, { - "filter_only": False, "name": "default.hard_hat.hire_date", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "timestamp", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.last_name", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.manager", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.postal_code", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.state", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat.title", "node_name": "default.hard_hat", "node_display_name": "Hard Hat", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.address", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.birth_date", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "timestamp", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.city", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.contractor_id", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.country", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.first_name", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.hard_hat_id", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": True, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": ["primary_key"], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.hire_date", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "timestamp", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.last_name", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.manager", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.postal_code", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.state", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.hard_hat_to_delete.title", "node_name": "default.hard_hat_to_delete", "node_display_name": "Hard Hat To Delete", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.municipality_dim.contact_name", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.municipality_dim.contact_title", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.municipality_dim.local_region", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.municipality_dim.municipality_id", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": True, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": ["primary_key"], }, { - "filter_only": False, "name": "default.municipality_dim.municipality_type_desc", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.municipality_dim.municipality_type_id", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.municipality_dim.state_id", "node_name": "default.municipality_dim", "node_display_name": "Municipality Dim", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.us_state.state_id", "node_name": "default.us_state", "node_display_name": "Us State", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact", "default.hard_hat"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.us_state.state_name", "node_name": "default.us_state", "node_display_name": "Us State", - "is_primary_key": False, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact", "default.hard_hat"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.us_state.state_region", "node_name": "default.us_state", "node_display_name": "Us State", - "is_primary_key": False, + "filter_only": False, "type": "int", "path": ["default.repair_orders_fact", "default.hard_hat"], - "is_hidden": False, + "properties": [], }, { - "filter_only": False, "name": "default.us_state.state_short", "node_name": "default.us_state", "node_display_name": "Us State", - "is_primary_key": True, + "filter_only": False, "type": "string", "path": ["default.repair_orders_fact", "default.hard_hat"], - "is_hidden": False, + "properties": ["primary_key"], }, ] @@ -558,34 +518,31 @@ async def test_read_metric( assert data["query"] == "SELECT COUNT(*) FROM parent" assert data["dimensions"] == [ { - "is_primary_key": False, + "filter_only": False, "name": "parent.ds", "node_display_name": "Parent", "node_name": "parent", "path": [], "type": "string", - "filter_only": False, - "is_hidden": False, + "properties": ["dimension"], }, { - "is_primary_key": False, + "filter_only": False, "name": "parent.foo", "node_display_name": "Parent", "node_name": "parent", "path": [], "type": "float", - "filter_only": False, - "is_hidden": False, + "properties": ["dimension"], }, { - "is_primary_key": False, + "filter_only": False, "name": "parent.user_id", "node_display_name": "Parent", "node_name": "parent", "path": [], "type": "int", - "filter_only": False, - "is_hidden": False, + "properties": ["dimension"], }, ] @@ -754,7 +711,6 @@ async def test_get_multi_link_dimensions( ) assert response.json()["dimensions"] == [ { - "is_primary_key": True, "name": "default.date_dim.dateint[birth_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -764,10 +720,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": True, "name": "default.date_dim.dateint[birth_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -777,10 +732,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": True, "name": "default.date_dim.dateint[residence_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -790,10 +744,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": True, "name": "default.date_dim.dateint[residence_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -803,10 +756,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": False, "name": "default.date_dim.day[birth_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -816,10 +768,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.day[birth_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -829,10 +780,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.day[residence_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -842,10 +792,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.day[residence_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -855,10 +804,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.month[birth_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -868,10 +816,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.month[birth_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -881,10 +828,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.month[residence_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -894,10 +840,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.month[residence_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -907,10 +852,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.year[birth_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -920,10 +864,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.year[birth_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -933,10 +876,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.year[residence_country->formation_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -946,10 +888,9 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.date_dim.year[residence_country->last_election_date]", "node_display_name": "Date Dim", "node_name": "default.date_dim", @@ -959,127 +900,115 @@ async def test_get_multi_link_dimensions( ], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": True, "name": "default.special_country_dim.country_code[birth_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.birth_country"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": True, "name": "default.special_country_dim.country_code[residence_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.residence_country"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": False, "name": "default.special_country_dim.formation_date[birth_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.birth_country"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.special_country_dim.formation_date[residence_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.residence_country"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.special_country_dim.last_election_date[birth_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.birth_country"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.special_country_dim.last_election_date[residence_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.residence_country"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.special_country_dim.name[birth_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.birth_country"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.special_country_dim.name[residence_country]", "node_display_name": "Special Country Dim", "node_name": "default.special_country_dim", "path": ["default.user_dim.residence_country"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.user_dim.age", "node_display_name": "User Dim", "node_name": "default.user_dim", "path": [], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.user_dim.birth_country", "node_display_name": "User Dim", "node_name": "default.user_dim", "path": [], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.user_dim.residence_country", "node_display_name": "User Dim", "node_name": "default.user_dim", "path": [], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": True, "name": "default.user_dim.user_id", "node_display_name": "User Dim", "node_name": "default.user_dim", "path": [], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, ] diff --git a/datajunction-server/tests/api/nodes_test.py b/datajunction-server/tests/api/nodes_test.py index 5f7d131a2..8bdbe7b35 100644 --- a/datajunction-server/tests/api/nodes_test.py +++ b/datajunction-server/tests/api/nodes_test.py @@ -1005,94 +1005,85 @@ async def test_deleting_linked_dimension( assert response.status_code in (200, 201) assert response.json()["dimensions"] == [ { - "is_primary_key": False, "name": "default.us_users.age", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.country", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.created_at", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "timestamp", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.full_name", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.gender", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": True, "name": "default.us_users.id", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": False, "name": "default.us_users.post_processing_timestamp", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "timestamp", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.preferred_language", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.secret_number", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "float", "filter_only": False, - "is_hidden": False, + "properties": [], }, ] @@ -1130,94 +1121,85 @@ async def test_deleting_linked_dimension( assert response.status_code in (200, 201) assert response.json()["dimensions"] == [ { - "is_primary_key": False, "name": "default.us_users.age", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.country", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.created_at", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "timestamp", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.full_name", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.gender", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": True, "name": "default.us_users.id", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": False, "name": "default.us_users.post_processing_timestamp", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "timestamp", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.preferred_language", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "string", "filter_only": False, - "is_hidden": False, + "properties": [], }, { - "is_primary_key": False, "name": "default.us_users.secret_number", "node_display_name": "Us Users", "node_name": "default.us_users", "path": ["default.messages"], "type": "float", "filter_only": False, - "is_hidden": False, + "properties": [], }, ] # The metric should still be VALID @@ -3639,17 +3621,17 @@ async def test_set_column_hidden( response = await client_with_basic.get( "/nodes/basic.dimension.users/dimensions", ) - column_attributes = {col["name"]: col["is_hidden"] for col in response.json()} + column_attributes = {col["name"]: col["properties"] for col in response.json()} assert column_attributes == { - "basic.dimension.users.age": False, - "basic.dimension.users.country": False, - "basic.dimension.users.created_at": False, - "basic.dimension.users.full_name": False, - "basic.dimension.users.gender": False, - "basic.dimension.users.id": False, - "basic.dimension.users.post_processing_timestamp": False, - "basic.dimension.users.preferred_language": False, - "basic.dimension.users.secret_number": True, + "basic.dimension.users.age": [], + "basic.dimension.users.country": [], + "basic.dimension.users.created_at": [], + "basic.dimension.users.full_name": [], + "basic.dimension.users.gender": [], + "basic.dimension.users.id": ["primary_key"], + "basic.dimension.users.post_processing_timestamp": [], + "basic.dimension.users.preferred_language": [], + "basic.dimension.users.secret_number": ["hidden"], } @pytest.mark.asyncio @@ -5215,97 +5197,87 @@ async def test_list_dimension_attributes(client_with_roads: AsyncClient) -> None assert response.json() == [ { "filter_only": False, - "is_primary_key": True, "name": "default.regional_level_agg.order_day", "node_display_name": "Regional Level Agg", "node_name": "default.regional_level_agg", "path": [], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": False, - "is_primary_key": True, "name": "default.regional_level_agg.order_month", "node_display_name": "Regional Level Agg", "node_name": "default.regional_level_agg", "path": [], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": False, - "is_primary_key": True, "name": "default.regional_level_agg.order_year", "node_display_name": "Regional Level Agg", "node_name": "default.regional_level_agg", "path": [], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": False, - "is_primary_key": True, "name": "default.regional_level_agg.state_name", "node_display_name": "Regional Level Agg", "node_name": "default.regional_level_agg", "path": [], "type": "string", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": False, - "is_primary_key": True, "name": "default.regional_level_agg.us_region_id", "node_display_name": "Regional Level Agg", "node_name": "default.regional_level_agg", "path": [], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": True, - "is_primary_key": True, "name": "default.repair_order.repair_order_id", "node_display_name": "Repair Order", "node_name": "default.repair_order", "path": ["default.repair_orders"], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": True, - "is_primary_key": True, "name": "default.dispatcher.dispatcher_id", "node_display_name": "Dispatcher", "node_name": "default.dispatcher", "path": ["default.repair_orders"], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": True, - "is_primary_key": True, "name": "default.repair_order.repair_order_id", "node_display_name": "Repair Order", "node_name": "default.repair_order", "path": ["default.repair_order_details"], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": True, - "is_primary_key": True, "name": "default.contractor.contractor_id", "node_display_name": "Contractor", "node_name": "default.contractor", "path": ["default.repair_type"], "type": "int", - "is_hidden": False, + "properties": ["primary_key"], }, { "filter_only": True, - "is_primary_key": True, "name": "default.us_state.state_short", "node_display_name": "Us State", "node_name": "default.us_state", @@ -5313,7 +5285,7 @@ async def test_list_dimension_attributes(client_with_roads: AsyncClient) -> None "default.contractors", ], "type": "string", - "is_hidden": False, + "properties": ["primary_key"], }, ] diff --git a/datajunction-server/tests/api/sql_test.py b/datajunction-server/tests/api/sql_test.py index 0b9091535..7cf68851a 100644 --- a/datajunction-server/tests/api/sql_test.py +++ b/datajunction-server/tests/api/sql_test.py @@ -3993,24 +3993,22 @@ async def test_filter_on_source_nodes( ) assert response.json() == [ { - "is_primary_key": True, "name": "default.events_agg.user_id", "node_display_name": "Events Agg", "node_name": "default.events_agg", "path": [], "type": "int", "filter_only": False, - "is_hidden": False, + "properties": ["primary_key"], }, { - "is_primary_key": True, "name": "default.event_date.dateint", "node_display_name": "Event Date", "node_name": "default.event_date", "path": ["default.events"], "type": "int", "filter_only": True, - "is_hidden": False, + "properties": ["primary_key"], }, ] diff --git a/datajunction-server/tests/sql/dag_test.py b/datajunction-server/tests/sql/dag_test.py index 3dea2ee80..e5b85335c 100644 --- a/datajunction-server/tests/sql/dag_test.py +++ b/datajunction-server/tests/sql/dag_test.py @@ -92,21 +92,19 @@ async def test_get_dimensions(session: AsyncSession, current_user: User) -> None name="B.attribute", node_name="B", node_display_name="B", - is_primary_key=False, + properties=[], type="string", path=["A.b_id"], filter_only=False, - is_hidden=False, ), DimensionAttributeOutput( name="B.id", node_name="B", node_display_name="B", - is_primary_key=False, + properties=[], type="int", path=["A.b_id"], filter_only=False, - is_hidden=False, ), ] From f22b5c2f4cebebb5f0436b14ac684ecbc685a7e7 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 21:39:37 -0800 Subject: [PATCH 06/14] Change DJ UI to depend on dimension attribute properties --- .../CubeBuilderPage/DimensionsSelect.jsx | 4 +- .../CubeBuilderPage/__tests__/index.test.jsx | 16 +++---- .../pages/NodePage/NodeDependenciesTab.jsx | 2 +- .../app/pages/NodePage/NodeValidateTab.jsx | 4 +- .../__tests__/NodeDependenciesTab.test.jsx | 8 ++-- datajunction-ui/src/mocks/mockNodes.jsx | 42 +++++++++---------- 6 files changed, 38 insertions(+), 38 deletions(-) diff --git a/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx b/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx index 842cd58d5..8ce25ca17 100644 --- a/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx +++ b/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx @@ -37,7 +37,7 @@ export const DimensionsSelect = ({ cube }) => { value: cubeDim.node_name + '.' + cubeDim.name, label: labelize(cubeDim.name) + - (cubeDim.is_primary_key ? ' (PK)' : ''), + (cubeDim.properties.includes("primary_key") ? ' (PK)' : ''), }; }); setDefaultDimensions(cubeDimensions); @@ -118,7 +118,7 @@ export const DimensionsSelect = ({ cube }) => { value: dim.name, label: labelize(dim.name.split('.').slice(-1)[0]) + - (dim.is_primary_key ? ' (PK)' : ''), + (dim.properties.includes("primary_key") ? ' (PK)' : ''), }; }); // diff --git a/datajunction-ui/src/app/pages/CubeBuilderPage/__tests__/index.test.jsx b/datajunction-ui/src/app/pages/CubeBuilderPage/__tests__/index.test.jsx index 64b41d507..32a5642aa 100644 --- a/datajunction-ui/src/app/pages/CubeBuilderPage/__tests__/index.test.jsx +++ b/datajunction-ui/src/app/pages/CubeBuilderPage/__tests__/index.test.jsx @@ -106,7 +106,7 @@ const mockCommonDimensions = [ type: 'timestamp', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -118,7 +118,7 @@ const mockCommonDimensions = [ type: 'timestamp', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: true, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -130,7 +130,7 @@ const mockCommonDimensions = [ type: 'int', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -142,7 +142,7 @@ const mockCommonDimensions = [ type: 'int', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -154,7 +154,7 @@ const mockCommonDimensions = [ type: 'int', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -166,7 +166,7 @@ const mockCommonDimensions = [ type: 'int', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -178,7 +178,7 @@ const mockCommonDimensions = [ type: 'int', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', @@ -190,7 +190,7 @@ const mockCommonDimensions = [ type: 'int', node_name: 'default.date_dim', node_display_name: 'Date', - is_primary_key: false, + properties: [], path: [ 'default.repair_order_details.repair_order_id', 'default.repair_order.hard_hat_id', diff --git a/datajunction-ui/src/app/pages/NodePage/NodeDependenciesTab.jsx b/datajunction-ui/src/app/pages/NodePage/NodeDependenciesTab.jsx index 2cc4cbbff..015733629 100644 --- a/datajunction-ui/src/app/pages/NodePage/NodeDependenciesTab.jsx +++ b/datajunction-ui/src/app/pages/NodePage/NodeDependenciesTab.jsx @@ -98,7 +98,7 @@ export function NodeDimensionsList({ rawDimensions }) { value: dim.name, label: labelize(dim.name.split('.').slice(-1)[0]) + - (dim.is_primary_key ? ' (PK)' : ''), + (dim.properties.includes("primary_key") ? ' (PK)' : ''), }; }); return ( diff --git a/datajunction-ui/src/app/pages/NodePage/NodeValidateTab.jsx b/datajunction-ui/src/app/pages/NodePage/NodeValidateTab.jsx index d9edc3610..467650317 100644 --- a/datajunction-ui/src/app/pages/NodePage/NodeValidateTab.jsx +++ b/datajunction-ui/src/app/pages/NodePage/NodeValidateTab.jsx @@ -73,7 +73,7 @@ export default function NodeValidateTab({ node, djClient }) { const dimensionsList = dimensions.flatMap(grouping => { const dimensionsInGroup = grouping[1]; return dimensionsInGroup - .filter(dim => dim.is_primary_key === true) + .filter(dim => dim.properties.includes("primary_key") === true) .map(dim => { return { value: dim.name, @@ -167,7 +167,7 @@ export default function NodeValidateTab({ node, djClient }) { const filters = dimensions.map(grouping => { const dimensionsInGroup = grouping[1]; const dimensionGroupOptions = dimensionsInGroup - .filter(dim => dim.is_primary_key === true) + .filter(dim => dim.properties.includes("primary_key") === true) .map(dim => { return { value: dim.name, diff --git a/datajunction-ui/src/app/pages/NodePage/__tests__/NodeDependenciesTab.test.jsx b/datajunction-ui/src/app/pages/NodePage/__tests__/NodeDependenciesTab.test.jsx index b1fc466cb..278e4f95f 100644 --- a/datajunction-ui/src/app/pages/NodePage/__tests__/NodeDependenciesTab.test.jsx +++ b/datajunction-ui/src/app/pages/NodePage/__tests__/NodeDependenciesTab.test.jsx @@ -95,7 +95,7 @@ describe('', () => { const mockDimensions = [ { - is_primary_key: false, + properties: [], name: 'default.dispatcher.company_name', node_display_name: 'Default: Dispatcher', node_name: 'default.dispatcher', @@ -103,7 +103,7 @@ describe('', () => { type: 'string', }, { - is_primary_key: true, + properties: ['primary_key'], name: 'default.dispatcher.dispatcher_id', node_display_name: 'Default: Dispatcher', node_name: 'default.dispatcher', @@ -111,7 +111,7 @@ describe('', () => { type: 'int', }, { - is_primary_key: false, + properties: [], name: 'default.hard_hat.city', node_display_name: 'Default: Hard Hat', node_name: 'default.hard_hat', @@ -119,7 +119,7 @@ describe('', () => { type: 'string', }, { - is_primary_key: true, + properties: ['primary_key'], name: 'default.hard_hat.hard_hat_id', node_display_name: 'Default: Hard Hat', node_name: 'default.hard_hat', diff --git a/datajunction-ui/src/mocks/mockNodes.jsx b/datajunction-ui/src/mocks/mockNodes.jsx index 9bf725a9e..8f5017015 100644 --- a/datajunction-ui/src/mocks/mockNodes.jsx +++ b/datajunction-ui/src/mocks/mockNodes.jsx @@ -964,7 +964,7 @@ export const mocks = { name: 'default.date_dim.dateint', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: true, + properties: ['primary_key'], type: 'timestamp', path: [ 'default.repair_order_details.repair_order_id', @@ -976,7 +976,7 @@ export const mocks = { name: 'default.date_dim.dateint', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: true, + properties: ['primary_key'], type: 'timestamp', path: [ 'default.repair_order_details.repair_order_id', @@ -988,7 +988,7 @@ export const mocks = { name: 'default.date_dim.day', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1000,7 +1000,7 @@ export const mocks = { name: 'default.date_dim.day', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1012,7 +1012,7 @@ export const mocks = { name: 'default.date_dim.month', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1024,7 +1024,7 @@ export const mocks = { name: 'default.date_dim.month', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1036,7 +1036,7 @@ export const mocks = { name: 'default.date_dim.year', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1048,7 +1048,7 @@ export const mocks = { name: 'default.date_dim.year', node_name: 'default.date_dim', node_display_name: 'Date Dim', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1060,7 +1060,7 @@ export const mocks = { name: 'default.hard_hat.address', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1071,7 +1071,7 @@ export const mocks = { name: 'default.hard_hat.birth_date', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'date', path: [ 'default.repair_order_details.repair_order_id', @@ -1082,7 +1082,7 @@ export const mocks = { name: 'default.hard_hat.city', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1093,7 +1093,7 @@ export const mocks = { name: 'default.hard_hat.contractor_id', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1104,7 +1104,7 @@ export const mocks = { name: 'default.hard_hat.country', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1115,7 +1115,7 @@ export const mocks = { name: 'default.hard_hat.first_name', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1126,7 +1126,7 @@ export const mocks = { name: 'default.hard_hat.hard_hat_id', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: true, + properties: ['primary_key'], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1137,7 +1137,7 @@ export const mocks = { name: 'default.hard_hat.hire_date', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'date', path: [ 'default.repair_order_details.repair_order_id', @@ -1148,7 +1148,7 @@ export const mocks = { name: 'default.hard_hat.last_name', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1159,7 +1159,7 @@ export const mocks = { name: 'default.hard_hat.manager', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'int', path: [ 'default.repair_order_details.repair_order_id', @@ -1170,7 +1170,7 @@ export const mocks = { name: 'default.hard_hat.postal_code', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1181,7 +1181,7 @@ export const mocks = { name: 'default.hard_hat.state', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', @@ -1192,7 +1192,7 @@ export const mocks = { name: 'default.hard_hat.title', node_name: 'default.hard_hat', node_display_name: 'Hard hat', - is_primary_key: false, + properties: [], type: 'string', path: [ 'default.repair_order_details.repair_order_id', From e9dd3a956d70738bc36198a7963dfba9dbf399db Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Tue, 7 Jan 2025 22:02:34 -0800 Subject: [PATCH 07/14] Fix test --- .../src/app/pages/CubeBuilderPage/DimensionsSelect.jsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx b/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx index 8ce25ca17..92acaf632 100644 --- a/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx +++ b/datajunction-ui/src/app/pages/CubeBuilderPage/DimensionsSelect.jsx @@ -37,7 +37,7 @@ export const DimensionsSelect = ({ cube }) => { value: cubeDim.node_name + '.' + cubeDim.name, label: labelize(cubeDim.name) + - (cubeDim.properties.includes("primary_key") ? ' (PK)' : ''), + (cubeDim.properties?.includes('primary_key') ? ' (PK)' : ''), }; }); setDefaultDimensions(cubeDimensions); @@ -118,7 +118,7 @@ export const DimensionsSelect = ({ cube }) => { value: dim.name, label: labelize(dim.name.split('.').slice(-1)[0]) + - (dim.properties.includes("primary_key") ? ' (PK)' : ''), + (dim.properties?.includes('primary_key') ? ' (PK)' : ''), }; }); // From 6c6a658e800e06560dec0c6e576288123f196715 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Wed, 8 Jan 2025 06:47:51 -0800 Subject: [PATCH 08/14] Add resolver for handling lazy loading of dimension node on attributes --- .../datajunction_server/api/graphql/main.py | 8 +++--- .../api/graphql/resolvers/nodes.py | 28 +++++++++++++++++++ .../api/graphql/scalars/node.py | 17 +++-------- 3 files changed, 36 insertions(+), 17 deletions(-) diff --git a/datajunction-server/datajunction_server/api/graphql/main.py b/datajunction-server/datajunction_server/api/graphql/main.py index 6fe6eacac..f4c14b080 100644 --- a/datajunction-server/datajunction_server/api/graphql/main.py +++ b/datajunction-server/datajunction_server/api/graphql/main.py @@ -157,7 +157,7 @@ async def find_nodes_paginated( ) async def common_dimensions( self, - names: Annotated[ + nodes: Annotated[ Optional[List[str]], strawberry.argument( description="A list of metrics to find common dimensions for", @@ -169,13 +169,13 @@ async def common_dimensions( """ Return a list of common dimensions for a set of metrics. """ - nodes = await find_nodes_by(info, names) - dimensions = await get_shared_dimensions(info.context["session"], nodes) + nodes = await find_nodes_by(info, nodes) # type: ignore + dimensions = await get_shared_dimensions(info.context["session"], nodes) # type: ignore return [ DimensionAttribute( # type: ignore name=dim.name, attribute=dim.name.split(SEPARATOR)[-1], - properties=dim.attributes, + properties=dim.properties, type=dim.type, ) for dim in dimensions diff --git a/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py b/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py index 1af841feb..1160154f9 100644 --- a/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py +++ b/datajunction-server/datajunction_server/api/graphql/resolvers/nodes.py @@ -6,6 +6,7 @@ from sqlalchemy.orm import joinedload, selectinload from strawberry.types import Info +from datajunction_server.api.graphql.scalars.node import NodeName from datajunction_server.api.graphql.utils import extract_fields from datajunction_server.database.dimensionlink import DimensionLink from datajunction_server.database.node import Column, ColumnAttribute @@ -54,6 +55,33 @@ async def find_nodes_by( ) +async def get_node_by_name( + info: Info, + name: str, +) -> DBNode | NodeName | None: + """ + Retrieves a node by name. This function also tries to optimize the database + query by only retrieving joined-in fields if they were requested. + """ + session = info.context["session"] # type: ignore + fields = extract_fields(info) + if "name" in fields and len(fields) == 1: + return NodeName(name=name) # type: ignore + + options = load_node_options( + fields["nodes"] + if "nodes" in fields + else fields["edges"]["node"] + if "edges" in fields + else fields, + ) + return await DBNode.get_by_name( + session, + name=name, + options=options, + ) + + def load_node_options(fields): """ Based on the GraphQL query input fields, builds a list of node load options. diff --git a/datajunction-server/datajunction_server/api/graphql/scalars/node.py b/datajunction-server/datajunction_server/api/graphql/scalars/node.py index a2155fe95..c78ef3ad7 100644 --- a/datajunction-server/datajunction_server/api/graphql/scalars/node.py +++ b/datajunction-server/datajunction_server/api/graphql/scalars/node.py @@ -84,20 +84,11 @@ async def dimension_node(self, info: Info) -> "Node": if self._dimension_node: return self._dimension_node - dimension_node_name = self.name.rsplit(".", 1)[0] + # pylint: disable=import-outside-toplevel + from datajunction_server.api.graphql.resolvers.nodes import get_node_by_name - # Check if only the 'name' field is requested for dimension_node. Then we can avoid - # the DB call and return a minimal object - requested_fields = { - field.name.value for field in info.field_nodes[0].selection_set.selections - } - if "name" in requested_fields and len(requested_fields) == 1: - return NodeName(name=dimension_node_name) # type: ignore - - return await DBNode.get_by_name( # type: ignore - info.context["session"], - self.name.rsplit(".", 1)[0], - ) + dimension_node_name = self.name.rsplit(".", 1)[0] + return await get_node_by_name(info=info, name=dimension_node_name) # type: ignore @strawberry.type From a3f708ff8b10e1ab1f8491fc4114984f46eeff5b Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Wed, 8 Jan 2025 07:50:30 -0800 Subject: [PATCH 09/14] Add test for common dimensions query --- .../api/graphql/common_dimensions_test.py | 171 ++++++++++++++++++ 1 file changed, 171 insertions(+) create mode 100644 datajunction-server/tests/api/graphql/common_dimensions_test.py diff --git a/datajunction-server/tests/api/graphql/common_dimensions_test.py b/datajunction-server/tests/api/graphql/common_dimensions_test.py new file mode 100644 index 000000000..1264c1a6c --- /dev/null +++ b/datajunction-server/tests/api/graphql/common_dimensions_test.py @@ -0,0 +1,171 @@ +""" +Tests for the common dimensions query. +""" + +# pylint: disable=line-too-long +from typing import AsyncGenerator +from sqlalchemy import event + +import pytest +import pytest_asyncio +from httpx import AsyncClient +from sqlalchemy.ext.asyncio import AsyncSession + + +@pytest_asyncio.fixture +async def capture_queries(module__session: AsyncSession) -> AsyncGenerator[list[str], None]: + """ + Returns a list of strings, where each string represents a SQL statement + captured during the test. + """ + queries = [] + sync_engine = module__session.bind.sync_engine + + def before_cursor_execute(conn, cursor, statement, parameters, context, executemany): + queries.append(statement) + + # Attach event listener to capture queries + event.listen(sync_engine, "before_cursor_execute", before_cursor_execute) + + yield queries + + # Detach event listener after the test + event.remove(sync_engine, "before_cursor_execute", before_cursor_execute) + + +@pytest.mark.asyncio +async def test_get_common_dimensions( + module__client_with_roads: AsyncClient, + capture_queries: AsyncGenerator[list[str], None], +) -> None: + """ + Test getting common dimensions for a set of metrics + """ + + query = """ + { + commonDimensions(nodes: ["default.num_repair_orders", "default.avg_repair_price"]) { + name + type + attribute + properties + role + dimensionNode { + name + } + } + } + """ + + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + assert len(data["data"]["commonDimensions"]) == 40 + assert { + 'attribute': 'company_name', + 'dimensionNode': { + 'name': 'default.dispatcher', + }, + 'name': 'default.dispatcher.company_name', + 'properties': [], + 'role': None, + 'type': 'string', + } in data["data"]["commonDimensions"] + + assert { + 'attribute': 'dispatcher_id', + 'dimensionNode': { + 'name': 'default.dispatcher', + }, + 'name': 'default.dispatcher.dispatcher_id', + 'properties': [ + 'primary_key', + ], + 'role': None, + 'type': 'int', + } in data["data"]["commonDimensions"] + + assert len(capture_queries) <= 11 + + +@pytest.mark.asyncio +async def test_get_common_dimensions_with_full_dim_node( + module__client_with_roads: AsyncClient, + capture_queries: AsyncGenerator[list[str], None], +) -> None: + """ + Test getting common dimensions and requesting a full dimension node for each + """ + + query = """ + { + commonDimensions(nodes: ["default.num_repair_orders", "default.avg_repair_price"]) { + name + type + attribute + properties + role + dimensionNode { + name + current { + columns { + name + attributes { + attributeType{ + name + } + } + } + } + tags { + name + } + } + } + } + """ + + response = await module__client_with_roads.post("/graphql", json={"query": query}) + assert response.status_code == 200 + data = response.json() + assert len(data["data"]["commonDimensions"]) == 40 + + assert { +'attribute': 'state_name', +'dimensionNode': { + 'current': { + 'columns': [ + { + 'attributes': [], + 'name': 'state_id', + }, + { + 'attributes': [], + 'name': 'state_name', + }, + { + 'attributes': [ + { + 'attributeType': { + 'name': 'primary_key', + }, + }, + ], + 'name': 'state_short', + }, + { + 'attributes': [], + 'name': 'state_region', + }, + ], + }, + 'name': 'default.us_state', + 'tags': [], +}, +'name': 'default.us_state.state_name', +'properties': [], +'role': None, +'type': 'string', +} in data["data"]["commonDimensions"] + + assert len(capture_queries) > 200 From f8fea5c45a0188a6a237788242b48d3ffa88feea Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Wed, 8 Jan 2025 07:58:07 -0800 Subject: [PATCH 10/14] Add test for common dimensions query --- .../api/graphql/common_dimensions_test.py | 125 +++++++++--------- 1 file changed, 66 insertions(+), 59 deletions(-) diff --git a/datajunction-server/tests/api/graphql/common_dimensions_test.py b/datajunction-server/tests/api/graphql/common_dimensions_test.py index 1264c1a6c..70d29f94d 100644 --- a/datajunction-server/tests/api/graphql/common_dimensions_test.py +++ b/datajunction-server/tests/api/graphql/common_dimensions_test.py @@ -4,16 +4,18 @@ # pylint: disable=line-too-long from typing import AsyncGenerator -from sqlalchemy import event import pytest import pytest_asyncio from httpx import AsyncClient +from sqlalchemy import event from sqlalchemy.ext.asyncio import AsyncSession @pytest_asyncio.fixture -async def capture_queries(module__session: AsyncSession) -> AsyncGenerator[list[str], None]: +async def capture_queries( + module__session: AsyncSession, +) -> AsyncGenerator[list[str], None]: """ Returns a list of strings, where each string represents a SQL statement captured during the test. @@ -21,7 +23,14 @@ async def capture_queries(module__session: AsyncSession) -> AsyncGenerator[list[ queries = [] sync_engine = module__session.bind.sync_engine - def before_cursor_execute(conn, cursor, statement, parameters, context, executemany): + def before_cursor_execute( + _conn, + _cursor, + statement, + _parameters, + _context, + _executemany, + ): queries.append(statement) # Attach event listener to capture queries @@ -36,7 +45,7 @@ def before_cursor_execute(conn, cursor, statement, parameters, context, executem @pytest.mark.asyncio async def test_get_common_dimensions( module__client_with_roads: AsyncClient, - capture_queries: AsyncGenerator[list[str], None], + capture_queries: AsyncGenerator[list[str], None], # pylint: disable=redefined-outer-name ) -> None: """ Test getting common dimensions for a set of metrics @@ -62,36 +71,35 @@ async def test_get_common_dimensions( data = response.json() assert len(data["data"]["commonDimensions"]) == 40 assert { - 'attribute': 'company_name', - 'dimensionNode': { - 'name': 'default.dispatcher', + "attribute": "company_name", + "dimensionNode": { + "name": "default.dispatcher", }, - 'name': 'default.dispatcher.company_name', - 'properties': [], - 'role': None, - 'type': 'string', + "name": "default.dispatcher.company_name", + "properties": [], + "role": None, + "type": "string", } in data["data"]["commonDimensions"] assert { - 'attribute': 'dispatcher_id', - 'dimensionNode': { - 'name': 'default.dispatcher', - }, - 'name': 'default.dispatcher.dispatcher_id', - 'properties': [ - 'primary_key', - ], - 'role': None, - 'type': 'int', - } in data["data"]["commonDimensions"] - - assert len(capture_queries) <= 11 + "attribute": "dispatcher_id", + "dimensionNode": { + "name": "default.dispatcher", + }, + "name": "default.dispatcher.dispatcher_id", + "properties": [ + "primary_key", + ], + "role": None, + "type": "int", + } in data["data"]["commonDimensions"] + assert len(capture_queries) <= 11 # type: ignore @pytest.mark.asyncio async def test_get_common_dimensions_with_full_dim_node( module__client_with_roads: AsyncClient, - capture_queries: AsyncGenerator[list[str], None], + capture_queries: AsyncGenerator[list[str], None], # pylint: disable=redefined-outer-name ) -> None: """ Test getting common dimensions and requesting a full dimension node for each @@ -131,41 +139,40 @@ async def test_get_common_dimensions_with_full_dim_node( assert len(data["data"]["commonDimensions"]) == 40 assert { -'attribute': 'state_name', -'dimensionNode': { - 'current': { - 'columns': [ - { - 'attributes': [], - 'name': 'state_id', - }, - { - 'attributes': [], - 'name': 'state_name', - }, - { - 'attributes': [ + "attribute": "state_name", + "dimensionNode": { + "current": { + "columns": [ + { + "attributes": [], + "name": "state_id", + }, + { + "attributes": [], + "name": "state_name", + }, + { + "attributes": [ + { + "attributeType": { + "name": "primary_key", + }, + }, + ], + "name": "state_short", + }, { - 'attributeType': { - 'name': 'primary_key', - }, + "attributes": [], + "name": "state_region", }, ], - 'name': 'state_short', }, - { - 'attributes': [], - 'name': 'state_region', - }, - ], - }, - 'name': 'default.us_state', - 'tags': [], -}, -'name': 'default.us_state.state_name', -'properties': [], -'role': None, -'type': 'string', -} in data["data"]["commonDimensions"] - - assert len(capture_queries) > 200 + "name": "default.us_state", + "tags": [], + }, + "name": "default.us_state.state_name", + "properties": [], + "role": None, + "type": "string", + } in data["data"]["commonDimensions"] + assert len(capture_queries) > 200 # type: ignore From 1467bba0aeddc5404e67a06cc66b4756024d4e02 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Wed, 8 Jan 2025 08:43:17 -0800 Subject: [PATCH 11/14] Add test for common dimensions query --- .../tests/api/graphql/common_dimensions_test.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/datajunction-server/tests/api/graphql/common_dimensions_test.py b/datajunction-server/tests/api/graphql/common_dimensions_test.py index 70d29f94d..cfaa2bffc 100644 --- a/datajunction-server/tests/api/graphql/common_dimensions_test.py +++ b/datajunction-server/tests/api/graphql/common_dimensions_test.py @@ -45,7 +45,10 @@ def before_cursor_execute( @pytest.mark.asyncio async def test_get_common_dimensions( module__client_with_roads: AsyncClient, - capture_queries: AsyncGenerator[list[str], None], # pylint: disable=redefined-outer-name + capture_queries: AsyncGenerator[ # pylint: disable=redefined-outer-name + list[str], + None, + ], ) -> None: """ Test getting common dimensions for a set of metrics @@ -99,7 +102,10 @@ async def test_get_common_dimensions( @pytest.mark.asyncio async def test_get_common_dimensions_with_full_dim_node( module__client_with_roads: AsyncClient, - capture_queries: AsyncGenerator[list[str], None], # pylint: disable=redefined-outer-name + capture_queries: AsyncGenerator[ # pylint: disable=redefined-outer-name + list[str], + None, + ], ) -> None: """ Test getting common dimensions and requesting a full dimension node for each From ebce777b391f0e210a0ddb8f7b5c0cb0c2bdbfcf Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Thu, 9 Jan 2025 18:06:37 -0800 Subject: [PATCH 12/14] Update GQL commonDimensions query to work for all node types, not just metrics --- .../datajunction_server/api/graphql/main.py | 10 +++++----- datajunction-server/datajunction_server/sql/dag.py | 13 ++++++++++--- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/datajunction-server/datajunction_server/api/graphql/main.py b/datajunction-server/datajunction_server/api/graphql/main.py index f4c14b080..a1086e08a 100644 --- a/datajunction-server/datajunction_server/api/graphql/main.py +++ b/datajunction-server/datajunction_server/api/graphql/main.py @@ -12,7 +12,7 @@ from datajunction_server.api.graphql.scalars import Connection from datajunction_server.api.graphql.scalars.node import DimensionAttribute, Node from datajunction_server.models.node import NodeCursor, NodeType -from datajunction_server.sql.dag import get_shared_dimensions +from datajunction_server.sql.dag import get_common_dimensions from datajunction_server.utils import SEPARATOR, get_session, get_settings @@ -153,24 +153,24 @@ async def find_nodes_paginated( ) @strawberry.field( - description="Get common dimensions for the list of metrics", + description="Get common dimensions for one or more nodes", ) async def common_dimensions( self, nodes: Annotated[ Optional[List[str]], strawberry.argument( - description="A list of metrics to find common dimensions for", + description="A list of nodes to find common dimensions for", ), ] = None, *, info: Info, ) -> list[DimensionAttribute]: """ - Return a list of common dimensions for a set of metrics. + Return a list of common dimensions for a set of nodes. """ nodes = await find_nodes_by(info, nodes) # type: ignore - dimensions = await get_shared_dimensions(info.context["session"], nodes) # type: ignore + dimensions = await get_common_dimensions(info.context["session"], nodes) # type: ignore return [ DimensionAttribute( # type: ignore name=dim.name, diff --git a/datajunction-server/datajunction_server/sql/dag.py b/datajunction-server/datajunction_server/sql/dag.py index 7e18c2a02..19d18d567 100644 --- a/datajunction-server/datajunction_server/sql/dag.py +++ b/datajunction-server/datajunction_server/sql/dag.py @@ -607,7 +607,7 @@ async def get_shared_dimensions( metric_nodes: List[Node], ) -> List[DimensionAttributeOutput]: """ - Return a list of dimensions that are common between the nodes. + Return a list of dimensions that are common between the metric nodes. """ find_latest_node_revisions = [ and_( @@ -631,8 +631,15 @@ async def get_shared_dimensions( ) ) parents = list(set((await session.execute(statement)).scalars().all())) - common = await group_dimensions_by_name(session, parents[0]) - for node in parents[1:]: + return get_common_dimensions(session, parents) + + +async def get_common_dimensions(session: AsyncSession, nodes: list[Node]): + """ + Return a list of dimensions that are common between the nodes. + """ + common = await group_dimensions_by_name(session, nodes[0]) + for node in nodes[1:]: node_dimensions = await group_dimensions_by_name(session, node) # Merge each set of dimensions based on the name and path From cbbaf2301735bb7d798908f933c221ca98175418 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Thu, 9 Jan 2025 18:07:14 -0800 Subject: [PATCH 13/14] Fix --- datajunction-server/datajunction_server/sql/dag.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/datajunction-server/datajunction_server/sql/dag.py b/datajunction-server/datajunction_server/sql/dag.py index 19d18d567..b2f7bad17 100644 --- a/datajunction-server/datajunction_server/sql/dag.py +++ b/datajunction-server/datajunction_server/sql/dag.py @@ -631,7 +631,7 @@ async def get_shared_dimensions( ) ) parents = list(set((await session.execute(statement)).scalars().all())) - return get_common_dimensions(session, parents) + return await get_common_dimensions(session, parents) async def get_common_dimensions(session: AsyncSession, nodes: list[Node]): From 23bdaf047904eca13ecf9cf0a5ea99db5ea77bf9 Mon Sep 17 00:00:00 2001 From: Yian Shang Date: Thu, 9 Jan 2025 20:02:53 -0800 Subject: [PATCH 14/14] Fix retrieving dimensions for metric nodes --- .../datajunction_server/sql/dag.py | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/datajunction-server/datajunction_server/sql/dag.py b/datajunction-server/datajunction_server/sql/dag.py index b2f7bad17..9c4fc2da6 100644 --- a/datajunction-server/datajunction_server/sql/dag.py +++ b/datajunction-server/datajunction_server/sql/dag.py @@ -609,6 +609,17 @@ async def get_shared_dimensions( """ Return a list of dimensions that are common between the metric nodes. """ + parents = await get_metric_parents(session, metric_nodes) + return await get_common_dimensions(session, parents) + + +async def get_metric_parents( + session: AsyncSession, + metric_nodes: list[Node], +) -> list[Node]: + """ + Return a list of parent nodes of the metrics + """ find_latest_node_revisions = [ and_( NodeRevision.name == metric_node.name, @@ -630,14 +641,18 @@ async def get_shared_dimensions( ), ) ) - parents = list(set((await session.execute(statement)).scalars().all())) - return await get_common_dimensions(session, parents) + return list(set((await session.execute(statement)).scalars().all())) async def get_common_dimensions(session: AsyncSession, nodes: list[Node]): """ Return a list of dimensions that are common between the nodes. """ + metric_nodes = [node for node in nodes if node.type == NodeType.METRIC] + other_nodes = [node for node in nodes if node.type != NodeType.METRIC] + if metric_nodes: + nodes = list(set(other_nodes + await get_metric_parents(session, metric_nodes))) + common = await group_dimensions_by_name(session, nodes[0]) for node in nodes[1:]: node_dimensions = await group_dimensions_by_name(session, node)