diff --git a/docs/md/doc/semantic-table.md b/docs/md/doc/semantic-table.md index f9be7b25..cfeb1480 100644 --- a/docs/md/doc/semantic-table.md +++ b/docs/md/doc/semantic-table.md @@ -170,7 +170,7 @@ flights_with_deps.get_graph()['avg_distance_per_flight']['deps'] Use `graph_predecessors()` and `graph_successors()` to navigate dependencies: ```graph_traversal -from boring_semantic_layer import graph_predecessors, graph_successors +from boring_semantic_layer.graph_utils import graph_predecessors, graph_successors graph = flights_with_deps.get_graph() @@ -347,7 +347,8 @@ all_combinations = flights_st.join_cross(carriers) ### Requiring a Match -`join_one()` and `join_many()` only support `how="left"`. When a query should +`join_one()` and `join_many()` are always LEFT joins (there is no `how=` +parameter). When a query should require a match, make the row removal explicit with a filter on a non-nullable field from the right table: diff --git a/src/boring_semantic_layer/__init__.py b/src/boring_semantic_layer/__init__.py index 265784e3..d412138a 100644 --- a/src/boring_semantic_layer/__init__.py +++ b/src/boring_semantic_layer/__init__.py @@ -26,16 +26,8 @@ from .expr import ( SemanticModel, SemanticTable, - to_tagged, to_untagged, ) -from .graph_utils import ( - graph_bfs, - graph_invert, - graph_predecessors, - graph_successors, - graph_to_dict, -) from .ops import ( Dimension, Measure, @@ -44,7 +36,6 @@ ProfileError, get_connection, ) -from .serialization import from_tagged from .yaml import ( from_config, from_yaml, @@ -59,9 +50,7 @@ "SerializationError", "UnknownFieldError", "to_semantic_table", - "to_tagged", "to_untagged", - "from_tagged", "entity_dimension", "time_dimension", "SemanticModel", @@ -73,11 +62,6 @@ "MCPSemanticModel", "LangGraphBackend", "options", - "graph_bfs", - "graph_invert", - "graph_predecessors", - "graph_successors", - "graph_to_dict", "ProfileError", "get_connection", ] diff --git a/src/boring_semantic_layer/expr.py b/src/boring_semantic_layer/expr.py index b20f6ff8..a7c88623 100644 --- a/src/boring_semantic_layer/expr.py +++ b/src/boring_semantic_layer/expr.py @@ -165,6 +165,21 @@ def to_tagged(expr, aggregate_cache_storage=None): class SemanticTable(ir.Table): + @property + def name(self) -> str | None: + """The semantic model's name, or None where no single model applies. + + Defined on the base so every semantic expression answers `.name` + (aggregates, limits, and other derived shapes return None instead + of raising an AttributeError that blames ibis's Table). + """ + return getattr(self.op(), "name", None) + + @property + def description(self) -> str | None: + """The semantic model's description, or None.""" + return getattr(self.op(), "description", None) + def get_graph(self): """Get the dependency graph for this semantic table. @@ -247,6 +262,65 @@ def chart( create_chart = importlib.import_module("boring_semantic_layer.chart").chart return create_chart(self, spec=spec, backend=backend, format=format) + def query( + self, + dimensions: Sequence[str] | None = None, + measures: Sequence[str] | None = None, + filters: Sequence[dict | str | Callable] | None = None, + order_by: Sequence[tuple[str, str] | str] | None = None, + limit: int | None = None, + time_grain: str | None = None, + time_grains: dict[str, str] | None = None, + time_range: dict[str, str] | None = None, + having: Sequence[dict] | None = None, + ): + """Run a declarative (JSON-style) query against this semantic table. + + Available on every semantic expression (models, joins, filters, + grouped/aggregated results). See ``boring_semantic_layer.query.query`` + for parameter semantics and worked examples. + """ + return _query_module().query( + semantic_table=self, + dimensions=dimensions, + measures=measures, + filters=filters, + order_by=order_by, + limit=limit, + time_grain=time_grain, + time_grains=time_grains, + time_range=time_range, + having=having, + ) + + def compare_periods( + self, + dimensions: Sequence[str] | None = None, + measures: Sequence[str] | None = None, + current_time_range: dict[str, str] | None = None, + previous_time_range: dict[str, str] | None = None, + filters: Sequence[dict | str | Callable] | None = None, + time_dimension: str | None = None, + time_grain: str | None = None, + time_grains: dict[str, str] | None = None, + order_by: Sequence[tuple[str, str] | str] | None = None, + limit: int | None = None, + ): + """Compare measures across two time ranges (current/previous/delta).""" + return _query_module().compare_periods( + semantic_table=self, + dimensions=dimensions, + measures=measures, + current_time_range=current_time_range, + previous_time_range=previous_time_range, + filters=filters, + time_dimension=time_dimension, + time_grain=time_grain, + time_grains=time_grains, + order_by=order_by, + limit=limit, + ) + def filter(self, predicate: Callable) -> SemanticFilter: return SemanticFilter(source=self.op(), predicate=predicate) @@ -739,7 +813,6 @@ def _join_one_with_detected_grain( left_op, other, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str, ) -> SemanticJoin: """Construct ``join_one`` consistently for every semantic wrapper.""" other_op = other.op() if isinstance(other, SemanticTable) else other @@ -748,7 +821,7 @@ def _join_one_with_detected_grain( left=left_op, right=other_op, on=on, - how=how, + how="left", cardinality=cardinality, ) @@ -1087,7 +1160,6 @@ def join_one( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-one relationship semantics. @@ -1101,8 +1173,6 @@ def join_one( on: Join predicate. Accepts a lambda ``(left, right) -> bool``, a column name string, a Deferred ``_.col``, or a list of strings/Deferred for compound equi-joins. - how: Join type. Only ``"left"`` is supported. - Returns: SemanticJoin: The joined semantic model @@ -1111,13 +1181,12 @@ def join_one( >>> orders.join_one(customers, on=_.customer_id) >>> orders.join_one(customers, on=lambda o, c: o.customer_id == c.customer_id) """ - return _join_one_with_detected_grain(self.op(), other, on, how) + return _join_one_with_detected_grain(self.op(), other, on) def join_many( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-many relationship semantics. @@ -1126,8 +1195,6 @@ def join_many( on: Join predicate. Accepts a lambda ``(left, right) -> bool``, a column name string, a Deferred ``_.col``, or a list of strings/Deferred for compound equi-joins. - how: Join type. Only ``"left"`` is supported. - Returns: SemanticJoin: The joined semantic model @@ -1137,7 +1204,7 @@ def join_many( >>> customer.join_many(orders, on=lambda c, o: c.customer_id == o.customer_id) """ other_op = other.op() if isinstance(other, SemanticModel) else other - return SemanticJoin(left=self.op(), right=other_op, on=on, how=how, cardinality="many") + return SemanticJoin(left=self.op(), right=other_op, on=on, how="left", cardinality="many") def join_cross(self, other: SemanticModel) -> SemanticJoin: """Cross join (Cartesian product) with another semantic model. @@ -1161,7 +1228,7 @@ def join(self, *args, **kwargs): The generic join() method has been removed. Please use: - join_one(other, lambda l, r: condition) for one-to-one relationships - - join_many(other, lambda l, r: condition, how="left") for one-to-many relationships + - join_many(other, lambda l, r: condition) for one-to-many relationships - join_cross(other) for Cartesian product Examples: @@ -1218,58 +1285,6 @@ def __getitem__(self, key): f"'{key}' not found in dimensions, measures, or calculated measures", ) - def query( - self, - dimensions: Sequence[str] | None = None, - measures: Sequence[str] | None = None, - filters: list | None = None, - order_by: Sequence[tuple[str, str]] | None = None, - limit: int | None = None, - time_grain: str | None = None, - time_grains: dict[str, str] | None = None, - time_range: dict[str, str] | None = None, - having: list | None = None, - ): - return _query_module().query( - semantic_table=self, - dimensions=dimensions, - measures=measures, - filters=filters, - order_by=order_by, - limit=limit, - time_grain=time_grain, - time_grains=time_grains, - time_range=time_range, - having=having, - ) - - def compare_periods( - self, - dimensions: Sequence[str] | None = None, - measures: Sequence[str] | None = None, - current_time_range: dict[str, str] | None = None, - previous_time_range: dict[str, str] | None = None, - filters: list | None = None, - time_dimension: str | None = None, - time_grain: str | None = None, - time_grains: dict[str, str] | None = None, - order_by: Sequence[tuple[str, str]] | None = None, - limit: int | None = None, - ): - return _query_module().compare_periods( - semantic_table=self, - dimensions=dimensions, - measures=measures, - current_time_range=current_time_range, - previous_time_range=previous_time_range, - filters=filters, - time_dimension=time_dimension, - time_grain=time_grain, - time_grains=time_grains, - order_by=order_by, - limit=limit, - ) - class SemanticJoin(SemanticTable): def __init__( @@ -1409,31 +1424,6 @@ def calc_measures(self): def json_definition(self): return self.op().json_definition - def query( - self, - dimensions: list[str] | None = None, - measures: list[str] | None = None, - filters: dict[str, Any] | None = None, - order_by: list[str] | None = None, - limit: int | None = None, - time_grain: str | None = None, - time_grains: dict[str, str] | None = None, - time_range: dict[str, str] | None = None, - having: list | None = None, - ): - return _query_module().query( - semantic_table=self, - dimensions=dimensions, - measures=measures, - filters=filters, - order_by=order_by, - limit=limit, - time_grain=time_grain, - time_grains=time_grains, - time_range=time_range, - having=having, - ) - def as_table(self) -> SemanticModel: all_roots = _find_all_root_models(self.op()) return _build_semantic_model_from_roots(self.op().to_untagged(), all_roots) @@ -1480,23 +1470,21 @@ def join_one( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-one relationship semantics.""" - return _join_one_with_detected_grain(self.op(), other, on, how) + return _join_one_with_detected_grain(self.op(), other, on) def join_many( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-many relationship semantics.""" return SemanticJoin( left=self.op(), right=other.op() if isinstance(other, SemanticModel) else other, on=on, - how=how, + how="left", cardinality="many", ) @@ -1588,58 +1576,6 @@ def measures(self): def calc_measures(self): return dict(self.get_calculated_measures()) - def query( - self, - dimensions: Sequence[str] | None = None, - measures: Sequence[str] | None = None, - filters: list | None = None, - order_by: Sequence[tuple[str, str]] | None = None, - limit: int | None = None, - time_grain: str | None = None, - time_grains: dict[str, str] | None = None, - time_range: dict[str, str] | None = None, - having: list | None = None, - ): - return _query_module().query( - semantic_table=self, - dimensions=dimensions, - measures=measures, - filters=filters, - order_by=order_by, - limit=limit, - time_grain=time_grain, - time_grains=time_grains, - time_range=time_range, - having=having, - ) - - def compare_periods( - self, - dimensions: Sequence[str] | None = None, - measures: Sequence[str] | None = None, - current_time_range: dict[str, str] | None = None, - previous_time_range: dict[str, str] | None = None, - filters: list | None = None, - time_dimension: str | None = None, - time_grain: str | None = None, - time_grains: dict[str, str] | None = None, - order_by: Sequence[tuple[str, str]] | None = None, - limit: int | None = None, - ): - return _query_module().compare_periods( - semantic_table=self, - dimensions=dimensions, - measures=measures, - current_time_range=current_time_range, - previous_time_range=previous_time_range, - filters=filters, - time_dimension=time_dimension, - time_grain=time_grain, - time_grains=time_grains, - order_by=order_by, - limit=limit, - ) - def as_table(self) -> SemanticModel: all_roots = _find_all_root_models(self.op().source) return _build_semantic_model_from_roots(self.op().to_untagged(), all_roots) @@ -1688,23 +1624,21 @@ def join_one( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-one relationship semantics.""" - return _join_one_with_detected_grain(self.op(), other, on, how) + return _join_one_with_detected_grain(self.op(), other, on) def join_many( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-many relationship semantics.""" return SemanticJoin( left=self.op(), right=other.op() if isinstance(other, SemanticModel) else other, on=on, - how=how, + how="left", cardinality="many", ) @@ -2049,23 +1983,21 @@ def join_one( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-one relationship semantics.""" - return _join_one_with_detected_grain(self.op(), other, on, how) + return _join_one_with_detected_grain(self.op(), other, on) def join_many( self, other: SemanticModel, on: Callable[[Any, Any], ir.BooleanValue] | str | Deferred | Sequence[str | Deferred], - how: str = "left", ) -> SemanticJoin: """Join with one-to-many relationship semantics.""" return SemanticJoin( left=self.op(), right=other.op(), on=on, - how=how, + how="left", cardinality="many", ) diff --git a/src/boring_semantic_layer/ops/_core.py b/src/boring_semantic_layer/ops/_core.py index b6b1e3c7..fbc0672c 100644 --- a/src/boring_semantic_layer/ops/_core.py +++ b/src/boring_semantic_layer/ops/_core.py @@ -3652,29 +3652,6 @@ def description(self) -> str | None: def table(self): return self.to_untagged() - def query( - self, - dimensions: Sequence[str] | None = None, - measures: Sequence[str] | None = None, - filters: list | None = None, - order_by: Sequence[tuple[str, str]] | None = None, - limit: int | None = None, - time_grain: str | None = None, - time_range: dict[str, str] | None = None, - having: list | None = None, - ): - return _query_module().query( - semantic_table=self, - dimensions=dimensions, - measures=measures, - filters=filters, - order_by=order_by, - limit=limit, - time_grain=time_grain, - time_range=time_range, - having=having, - ) - def with_dimensions(self, **dims) -> SemanticTable: return _semantic_table( table=self.to_untagged(), @@ -3721,23 +3698,21 @@ def join_one( self, other: SemanticTable, on: Callable[[Any, Any], ir.BooleanValue], - how: str = "left", ): """Join with one-to-one relationship semantics (left outer join).""" - return _expr_module()._join_one_with_detected_grain(self, other, on, how) + return _expr_module()._join_one_with_detected_grain(self, other, on) def join_many( self, other: SemanticTable, on: Callable[[Any, Any], ir.BooleanValue], - how: str = "left", ): """Join with one-to-many relationship semantics.""" return _expr_module().SemanticJoin( left=self, right=other.op(), on=on, - how=how, + how="left", cardinality="many", ) diff --git a/src/boring_semantic_layer/query.py b/src/boring_semantic_layer/query.py index e49866b6..a7e25c4a 100644 --- a/src/boring_semantic_layer/query.py +++ b/src/boring_semantic_layer/query.py @@ -14,7 +14,8 @@ from ibis.common.collections import FrozenDict from toolz import curry -from .errors import QueryError, unwrap_or_raise +from .errors import QueryError, UnknownFieldError, suggest_kinded, unwrap_or_raise +from .fieldref import resolve_suffix from .safe_eval import safe_eval @@ -801,6 +802,30 @@ def query( order_by = _normalize_order_by(order_by, known_order_fields, expected_prefix=model_name) filters = list(filters or []) # Copy to avoid mutating input + # Fail fast on unknown names, with suggestions — before any compilation, + # so the user never sees a backend error listing physical columns. + raw_columns = set(getattr(result, "columns", ()) or ()) + for dim in dimensions: + if ( + dim not in known_dimensions + and dim not in raw_columns + and resolve_suffix(dim, known_dimensions, raw_columns) is None + ): + hint = suggest_kinded(dim, [("dimension", known_dimensions), ("column", raw_columns)]) + raise UnknownFieldError( + f"Unknown dimension {dim!r}. Declared dimensions: " + f"{sorted(known_dimensions)}.{' ' + hint if hint else ''}" + ) + for meas in measures or (): + if meas not in known_measures and resolve_suffix(meas, known_measures) is None: + hint = suggest_kinded( + meas, [("measure", known_measures), ("dimension", known_dimensions)] + ) + raise UnknownFieldError( + f"Unknown measure {meas!r}. Declared measures: " + f"{sorted(known_measures)}.{' ' + hint if hint else ''}" + ) + selected_fields = set(dimensions) | set(measures or []) produces_aggregate = bool(dimensions or measures) if produces_aggregate and order_by: diff --git a/src/boring_semantic_layer/serialization/reconstruct.py b/src/boring_semantic_layer/serialization/reconstruct.py index c2a71f7d..c5b0baec 100644 --- a/src/boring_semantic_layer/serialization/reconstruct.py +++ b/src/boring_semantic_layer/serialization/reconstruct.py @@ -392,9 +392,8 @@ def _reconstruct_join(metadata: dict, xorq_expr, source, context: BSLSerializati _validate_join_leaf(left_model, left_metadata, "left") _validate_join_leaf(right_model, right_metadata, "right") - # Payloads written before ``how`` was serialized must preserve left-side - # rows rather than silently treating the relationship as an inner join. - how = metadata.get("how", "left") + # ``how`` in stored payloads is informational: semantic joins are always + # LEFT joins (join_cross carries how="cross" on the op directly). # Default to "many" for payloads serialized before cardinality was # emitted — join_many is a safe superset of join_one behaviour, while # the reverse silently skips pre-aggregation. (Fixes #223.) @@ -406,7 +405,7 @@ def _reconstruct_join(metadata: dict, xorq_expr, source, context: BSLSerializati left=left_model.op() if hasattr(left_model, "op") else left_model, right=right_model.op() if hasattr(right_model, "op") else right_model, on=None, - how=how, + how="cross" if cardinality == "cross" else "left", cardinality=cardinality, ) @@ -418,7 +417,7 @@ def _reconstruct_join(metadata: dict, xorq_expr, source, context: BSLSerializati }.get(cardinality, "join_many") if join_method == "join_cross": return left_model.join_cross(right_model) - return getattr(left_model, join_method)(right_model, on=predicate, how=how) + return getattr(left_model, join_method)(right_model, on=predicate) # --------------------------------------------------------------------------- diff --git a/src/boring_semantic_layer/tests/soundness/test_join_namespacing_and_inline_totals.py b/src/boring_semantic_layer/tests/soundness/test_join_namespacing_and_inline_totals.py index f15ee342..b1f96156 100644 --- a/src/boring_semantic_layer/tests/soundness/test_join_namespacing_and_inline_totals.py +++ b/src/boring_semantic_layer/tests/soundness/test_join_namespacing_and_inline_totals.py @@ -127,7 +127,7 @@ def test_inline_count_star_on_joined_model(self, orders_customers): order_count=_.count(), pct=lambda t: t.count() / t.all(t.count()) * 100, ) - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") df = ( j.group_by("customers.region") .aggregate("orders.order_count", "orders.pct") @@ -149,7 +149,7 @@ def test_declared_dim_when_fact_table_has_no_dims(self, orders_customers): orders_tbl, cust_tbl = orders_customers customers = to_semantic_table(cust_tbl, name="customers").with_dimensions(cust_name=_.name) orders = to_semantic_table(orders_tbl, name="orders").with_measures(order_count=_.count()) - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") df = ( j.group_by("customers.cust_name") .aggregate("orders.order_count") @@ -180,7 +180,7 @@ def joined(self, orders_customers): def test_raw_right_column(self, joined): orders, customers = joined - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") df = ( j.group_by("customers.cust_id") .aggregate("orders.order_count") @@ -192,7 +192,7 @@ def test_raw_right_column(self, joined): def test_raw_left_column(self, joined): orders, customers = joined - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") df = j.group_by("orders.amount").aggregate("orders.order_count").execute() assert len(df) == 4 assert df["orders.order_count"].sum() == 4 @@ -200,7 +200,7 @@ def test_raw_left_column(self, joined): def test_raw_colliding_right_column(self, joined): """A raw right column colliding with a left column resolves right.""" orders, customers = joined - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") df = ( j.group_by("customers.name") .aggregate("orders.order_count") @@ -212,7 +212,7 @@ def test_raw_colliding_right_column(self, joined): def test_raw_prefixed_on_join_many_preagg(self, joined): orders, customers = joined - j = orders.join_many(customers, on="cust_id", how="left") + j = orders.join_many(customers, on="cust_id") df = ( j.group_by("customers.cust_id") .aggregate("orders.order_count") @@ -241,14 +241,14 @@ def test_declared_dimension_still_wins(self, orders_customers): region=lambda t: t.region.upper() ) orders = to_semantic_table(orders_tbl, name="orders").with_measures(order_count=_.count()) - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") df = j.group_by("customers.region").aggregate("orders.order_count").execute() assert sorted(df["customers.region"]) == ["EAST", "WEST"] def test_unknown_key_raises_semantic_error(self, joined): """Typos raise a semantic-layer error, not a physical-schema dump.""" orders, customers = joined - j = orders.join_one(customers, on="cust_id", how="left") + j = orders.join_one(customers, on="cust_id") with pytest.raises(KeyError) as excinfo: j.group_by("customers.regoin").aggregate("orders.order_count").execute() msg = str(excinfo.value) diff --git a/src/boring_semantic_layer/tests/test_join_pruning.py b/src/boring_semantic_layer/tests/test_join_pruning.py index 2c36c0a4..c6a05ff4 100644 --- a/src/boring_semantic_layer/tests/test_join_pruning.py +++ b/src/boring_semantic_layer/tests/test_join_pruning.py @@ -101,20 +101,19 @@ def _build_star_model(star_schema): return ( facts.join_one(dates, on=lambda f, d: f.date_id == d.date_id) - .join_one(stores, on=lambda f, s: f.store_id == s.store_id, how="left") - .join_one(items, on=lambda f, i: f.item_id == i.item_id, how="left") + .join_one(stores, on=lambda f, s: f.store_id == s.store_id) + .join_one(items, on=lambda f, i: f.item_id == i.item_id) ) @pytest.mark.parametrize("join_method", ["join_one", "join_many"]) -@pytest.mark.parametrize("how", ["inner", "right", "outer", "cross"]) -def test_semantic_joins_reject_non_left_join_types(star_schema, join_method, how): - """Semantic relationships preserve left rows; cross joins use join_cross().""" +def test_semantic_joins_have_no_how_parameter(star_schema, join_method): + """Semantic joins are always LEFT joins: the parameter no longer exists.""" facts = to_semantic_table(star_schema["facts"], name="reject_facts") dates = to_semantic_table(star_schema["dates"], name="reject_dates") - with pytest.raises(ValueError, match="only support how='left'"): - getattr(facts, join_method)(dates, on="date_id", how=how) + with pytest.raises(TypeError, match="how"): + getattr(facts, join_method)(dates, on="date_id", how="inner") def test_join_cross_remains_supported(star_schema): diff --git a/src/boring_semantic_layer/tests/test_rewrites_projection_pushdown.py b/src/boring_semantic_layer/tests/test_rewrites_projection_pushdown.py index 6fda2ac0..9c004db8 100644 --- a/src/boring_semantic_layer/tests/test_rewrites_projection_pushdown.py +++ b/src/boring_semantic_layer/tests/test_rewrites_projection_pushdown.py @@ -128,7 +128,7 @@ def test_projection_pushdown_after_join(wide_tables): ) # Join flights with aircraft - joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num, how="left") + joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num) # Query: group by origin, count flights # Should ONLY need: @@ -181,7 +181,7 @@ def test_projection_pushdown_with_dimension_from_right_table(wide_tables): ) # Join flights with aircraft - joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num, how="left") + joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num) # Query: group by manufacturer (from aircraft table), count flights # Should need: @@ -255,7 +255,7 @@ def test_projection_pushdown_counts_columns(wide_tables): ) # Join and query - joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num, how="left") + joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num) query = joined.group_by("flights.origin").aggregate("flight_count") sql = str(ibis.to_sql(to_untagged(query))) @@ -305,7 +305,7 @@ def test_projection_pushdown_multiple_dimensions(wide_tables): ) # Join tables - joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num, how="left") + joined = flights.join_many(aircraft, lambda f, a: f.tail_num == a.tail_num) # Query using dimensions from both tables query = joined.group_by("flights.origin", "aircraft.manufacturer").aggregate("flight_count") @@ -417,9 +417,9 @@ def test_projection_pushdown_three_way_join_all_notations(duckdb_con): ) # Three-way join using raw column access in predicates - joined = orders.join_many( - customers, lambda o, c: o.customer_id == c.customer_id, how="left" - ).join_many(items, lambda oc, i: oc.order_id == i.order_id, how="left") + joined = orders.join_many(customers, lambda o, c: o.customer_id == c.customer_id).join_many( + items, lambda oc, i: oc.order_id == i.order_id + ) # Test 1: Use bracket notation with prefixes in calculated measures result1 = ( diff --git a/src/boring_semantic_layer/tests/test_xorq_string_serialization.py b/src/boring_semantic_layer/tests/test_xorq_string_serialization.py index ffcd2a48..71c5b27b 100644 --- a/src/boring_semantic_layer/tests/test_xorq_string_serialization.py +++ b/src/boring_semantic_layer/tests/test_xorq_string_serialization.py @@ -1694,7 +1694,7 @@ def test_tagged_roundtrip_join_one_left_join(): name=Dimension(expr=lambda t: t.name, description="Name"), ) - joined = orders_st.join_one(products_st, on=lambda o, p: o.product_id == p.pid, how="left") + joined = orders_st.join_one(products_st, on=lambda o, p: o.product_id == p.pid) tagged = to_tagged(joined) reconstructed = from_tagged(tagged) diff --git a/src/boring_semantic_layer/yaml.py b/src/boring_semantic_layer/yaml.py index 137111cd..c09f1fa5 100644 --- a/src/boring_semantic_layer/yaml.py +++ b/src/boring_semantic_layer/yaml.py @@ -8,7 +8,7 @@ from ibis import _ from .api import to_semantic_table -from .errors import unwrap_or_raise +from .errors import DefinitionError, format_suggestions, unwrap_or_raise from .expr import SemanticModel, SemanticTable from .io import read_yaml_file from .ops import Dimension, Measure @@ -220,6 +220,12 @@ def _parse_joins( # Apply the join based on type join_type = join_config.get("type", "one") # Default to one-to-one how = join_config.get("how") or "left" + if how != "left": + raise DefinitionError( + f"Join {alias!r}: how={how!r} is not supported. Semantic joins " + "are always LEFT joins for soundness; filter afterwards for " + "inner-join semantics (e.g. filter: _.key.notnull())." + ) if join_type == "cross": # Cross join - no keys needed @@ -240,7 +246,6 @@ def make_join_condition(left_col, right_col): result_model = result_model.join_one( join_model, on=on_condition, - how=how, ) elif join_type == "many": left_on = join_config.get("left_on") @@ -258,7 +263,6 @@ def make_join_condition(left_col, right_col): result_model = result_model.join_many( join_model, on=on_condition, - how=how, ) else: raise ValueError(f"Invalid join type '{join_type}'. Must be 'one', 'many', or 'cross'") @@ -338,6 +342,74 @@ def _load_table_for_yaml_model( return tables, tables[table_name] +_MODEL_KEYS = frozenset( + { + "table", + "description", + "database", + "dimensions", + "measures", + "calculated_measures", + "joins", + "filter", + "profile", + } +) + +_VALID_TIME_GRAINS = frozenset( + { + "TIME_GRAIN_YEAR", + "TIME_GRAIN_QUARTER", + "TIME_GRAIN_MONTH", + "TIME_GRAIN_WEEK", + "TIME_GRAIN_DAY", + "TIME_GRAIN_HOUR", + "TIME_GRAIN_MINUTE", + "TIME_GRAIN_SECOND", + "year", + "quarter", + "month", + "week", + "day", + "hour", + "minute", + "second", + } +) + + +def _validate_model_config(name: str, model_config: Mapping[str, Any]) -> None: + """Reject unknown keys and invalid grains loudly, at load time. + + A typo like ``dimension:`` used to load silently as a model with zero + dimensions; every mistake now names the model, the key, and the + accepted spellings. + """ + unknown = sorted(set(model_config) - _MODEL_KEYS) + if unknown: + hints = "".join(format_suggestions(k, _MODEL_KEYS) for k in unknown) + raise DefinitionError( + f"Model {name!r} has unknown key(s) {unknown}. " + f"Accepted keys: {sorted(_MODEL_KEYS)}.{hints}" + ) + for section in ("dimensions", "measures", "calculated_measures"): + entries = model_config.get(section) or {} + if not isinstance(entries, Mapping): + raise DefinitionError( + f"Model {name!r}: {section!r} must be a mapping of name -> " + f"expression/config, got {type(entries).__name__}" + ) + for field_name, cfg in entries.items(): + if isinstance(cfg, Mapping): + grain = cfg.get("smallest_time_grain") + if grain is not None and grain not in _VALID_TIME_GRAINS: + raise DefinitionError( + f"Model {name!r}, {section[:-1]} {field_name!r}: invalid " + f"smallest_time_grain {grain!r}. Accepted values: " + f"{sorted(_VALID_TIME_GRAINS)}." + ) + + def from_config( config: Mapping[str, Any], tables: Mapping[str, Any] | None = None, @@ -410,9 +482,10 @@ def from_config( # First pass: create models for name, model_config in model_configs.items(): + _validate_model_config(name, model_config) table_name = model_config.get("table") if not table_name: - raise ValueError(f"Model '{name}' must specify 'table' field") + raise DefinitionError(f"Model '{name}' must specify 'table' field") # Load table if needed and verify it exists tables, table = _load_table_for_yaml_model(model_config, tables, table_name)