From 4e5d704d65daf3447d83fc6ad31302e180bc40d6 Mon Sep 17 00:00:00 2001 From: Mirko <2361009+mischuh@users.noreply.github.com> Date: Wed, 22 Jul 2026 20:59:58 +0200 Subject: [PATCH] fix(compiler): thread each ratio component's own population_filter into its leaf For composable_post_agg metrics (ratio/weighted_avg), _compile_composite only ever read the ratio metric's own population_filter and applied it to both numerator and denominator leaves. A population_filter declared on a component measure itself (e.g. total_repair_cost or damage_count) was silently ignored, so the divisor and dividend were compiled unfiltered. --- .gitignore | 1 + LICENSE.md | 122 ++++++++++------------- canonic/compiler/strategies/composite.py | 29 +++++- tests/compiler/test_population_filter.py | 49 +++++++++ 4 files changed, 126 insertions(+), 75 deletions(-) diff --git a/.gitignore b/.gitignore index 83441d9..872338c 100644 --- a/.gitignore +++ b/.gitignore @@ -222,5 +222,6 @@ __marimo__/ .DS_Store specs/ +vids/ .vscode/settings.json .claude/settings.json diff --git a/LICENSE.md b/LICENSE.md index 044e182..5fca271 100644 --- a/LICENSE.md +++ b/LICENSE.md @@ -1,72 +1,50 @@ -Business Source License 1.1 - -Parameters - -Licensor: Mirko Schuh -Licensed Work: Canonic - including any software source code, documentation, or other materials - made available under this License. -Additional Use Grant: - You may use the Licensed Work for non-commercial, personal, evaluation, - educational, or developmental purposes. - - You may NOT use the Licensed Work for any commercial purpose, including but - not limited to: - - Offering the Licensed Work, or any modified version thereof, as a commercial, - hosted, managed, or SaaS (Software-as-a-Service) platform. - - Integrating the Licensed Work into a commercial product, service, or solution - offered to third parties. - - Selling, licensing, renting, or leasing the Licensed Work or any part thereof - for a fee, regardless of the size or revenue of your organization. - - Any commercial use not expressly permitted above requires a separate commercial - license from the Licensor. -Change Date: July 17, 2030 (4 years after the release of this version) -Change License: Apache License, Version 2.0 - ------------------------------------------------------------------------------------------ - -TERMS AND CONDITIONS - -1. Grant of License - -Subject to the terms and conditions of this License, Licensor hereby grants you a -non-exclusive, worldwide, royalty-free, non-transferable, and non-sublicensable license -to copy, modify, create derivative works of, and distribute the Licensed Work, solely -as permitted by the Additional Use Grant, if any. - -2. Change to Open Source License - -On the Change Date, or on such earlier date as Licensor may designate by public notice -or in writing, the License granted under Section 1 will automatically terminate, and the -Licensed Work will automatically be licensed under the Change License. Any use of the -Licensed Work after the Change Date will be governed solely by the terms of the Change -License. - -3. Notices - -You must include a copy of this License, including the Parameters above, in any copy of -the Licensed Work that you make or distribute, and in any derivative work of the Licensed -Work that you create. You must retain all copyright, trademark, patent, and attribution -notices from the Licensed Work in any copies or derivative works. - -4. Intellectual Property Rights - -Except as expressly provided in this License, Licensor retains all right, title, and -interest in and to the Licensed Work, including all intellectual property rights. - -5. Disclaimer of Warranties - -THE LICENSED WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, -INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR -PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE -FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR -OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE LICENSED WORK OR THE USE OR -OTHER DEALINGS IN THE LICENSED WORK. - -6. Limitation of Liability - -TO THE MAXIMUM EXTENT PERMITTED BY APPLICABLE LAW, IN NO EVENT WILL LICENSOR BE LIABLE -FOR ANY INDIRECT, SPECIAL, INCIDENTAL, CONSEQUENTIAL, OR EXEMPLARY DAMAGES ARISING OUT -OF OR IN CONNECTION WITH THIS LICENSE OR THE USE OF THE LICENSED WORK, EVEN IF LICENSOR -HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. \ No newline at end of file +# Business Source License 1.1 + +## Parameters + +**Licensor:** mischuh + +**Licensed Work:** Canonic, including any software source code, documentation, or other materials made available under this License. + +**Additional Use Grant:** +You may use the Licensed Work for non-commercial, personal, evaluation, educational, or developmental purposes. + +You may **NOT** use the Licensed Work for any commercial purpose, including but not limited to: + +- Offering the Licensed Work, or any modified version thereof, as a commercial, hosted, managed, or SaaS (Software-as-a-Service) platform. +- Integrating the Licensed Work into a commercial product, service, or solution offered to third parties. +- Selling, licensing, renting, or leasing the Licensed Work or any part thereof for a fee, regardless of the size or revenue of your organization. + +Any commercial use not expressly permitted above requires a separate commercial license from the Licensor. + +**Change Date:** July 17, 2030 (4 years after the release of this version) + +**Change License:** Apache License, Version 2.0 + +--- + +## Terms and Conditions + +### 1. Grant of License + +Subject to the terms and conditions of this License, Licensor hereby grants you a non-exclusive, worldwide, royalty-free, non-transferable, and non-sublicensable license to copy, modify, create derivative works of, and distribute the Licensed Work, solely as permitted by the Additional Use Grant, if any. + +### 2. Change to Open Source License + +On the Change Date, or on such earlier date as Licensor may designate by public notice or in writing, the License granted under Section 1 will automatically terminate, and the Licensed Work will automatically be licensed under the Change License. Any use of the Licensed Work after the Change Date will be governed solely by the terms of the Change License. + +### 3. Notices + +You must include a copy of this License, including the Parameters above, in any copy of the Licensed Work that you make or distribute, and in any derivative work of the Licensed Work that you create. You must retain all copyright, trademark, patent, and attribution notices from the Licensed Work in any copies or derivative works. + +### 4. Intellectual Property Rights + +Except as expressly provided in this License, Licensor retains all right, title, and interest in and to the Licensed Work, including all intellectual property rights. + +### 5. Disclaimer of Warranties + +THE LICENSED WORK IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE LICENSED WORK OR THE USE OR OTHER DEALINGS IN THE LICENSED WORK. + +### 6. Limitation of Liability + +TO THE MAXIMUM EXTENT PERMITTED BY APPLICABLE LAW, IN NO EVENT WILL LICENSOR BE LIABLE FOR ANY INDIRECT, SPECIAL, INCIDENTAL, CONSEQUENTIAL, OR EXEMPLARY DAMAGES ARISING OUT OF OR IN CONNECTION WITH THIS LICENSE OR THE USE OF THE LICENSED WORK, EVEN IF LICENSOR HAS BEEN ADVISED OF THE POSSIBILITY OF SUCH DAMAGES. diff --git a/canonic/compiler/strategies/composite.py b/canonic/compiler/strategies/composite.py index 005a773..c14a2c1 100644 --- a/canonic/compiler/strategies/composite.py +++ b/canonic/compiler/strategies/composite.py @@ -245,6 +245,19 @@ def _build_leaf_select( return select +def _combine_population_filters(*filters: str | None) -> str | None: + """AND together a composite-level and a component-level population_filter (ยง4.5). + + Both the ratio metric itself and each of its numerator/denominator building blocks + may declare a population_filter; a leaf must honor whichever of its own owner's + filter and the composite's filter are present, not just one of the two. + """ + present = [f for f in filters if f] + if not present: + return None + return " AND ".join(f"({f})" for f in present) + + def _compile_composite( query: SemanticQuery, composite: ResolverBinding, @@ -265,9 +278,19 @@ def _compile_composite( adapter = adapter_for(dialect) queried_name = query.metrics[0] - pop_filter = composite.binding.canonical.population_filter - num_plan = _plan_leaf(components.numerator, query, resolver, sources_by_name, "n", pop_filter) - den_plan = _plan_leaf(components.denominator, query, resolver, sources_by_name, "d", pop_filter) + composite_pop_filter = composite.binding.canonical.population_filter + num_pop_filter = _combine_population_filters( + composite_pop_filter, components.numerator.binding.canonical.population_filter + ) + den_pop_filter = _combine_population_filters( + composite_pop_filter, components.denominator.binding.canonical.population_filter + ) + num_plan = _plan_leaf( + components.numerator, query, resolver, sources_by_name, "n", num_pop_filter + ) + den_plan = _plan_leaf( + components.denominator, query, resolver, sources_by_name, "d", den_pop_filter + ) dim_names = num_plan.dim_names diff --git a/tests/compiler/test_population_filter.py b/tests/compiler/test_population_filter.py index e39527b..db77312 100644 --- a/tests/compiler/test_population_filter.py +++ b/tests/compiler/test_population_filter.py @@ -161,6 +161,55 @@ def test_ac1_population_filter_model_roundtrip() -> None: assert ref.population_filter == "severity IN ('major', 'moderate')" +def test_ratio_population_filter_on_component_measures( + damages: SemanticSource, +) -> None: + """population_filter declared on the component measures (not the ratio itself) applies. + + Regression for the bug where a filter set on the numerator's/denominator's own + CanonicalRef was silently dropped because _compile_composite only ever read the + ratio metric's own population_filter for both leaves. Uses two distinct filters, + one per component, to prove each leaf gets its own filter rather than one leaking + into the other or both being dropped. + """ + total_cost_binding = MetricBinding( + metric="total_repair_cost", + canonical=CanonicalRef( + source="damages", + measure="total_repair_cost", + population_filter="severity IN ('major', 'moderate')", + ), + ) + damage_count_binding = MetricBinding( + metric="damage_count", + canonical=CanonicalRef( + source="damages", + measure="damage_count", + population_filter="reported_month = '2024-01'", + ), + ) + avg_costs_binding = MetricBinding( + metric="avg_repair_costs", + canonical=CanonicalRef( + kind=BindingKind.RATIO, + numerator="total_repair_cost", + denominator="damage_count", + ), + ) + resolver = ContractResolver( + bindings=[avg_costs_binding, total_cost_binding, damage_count_binding], guardrails=[] + ) + result = compile(SemanticQuery(metrics=["avg_repair_costs"]), resolver, [damages]) + _parse_ok(result.sql) + sql_lower = result.sql.lower() + # Each component's own filter must land in its own leaf CTE. + assert "severity" in sql_lower + assert "major" in sql_lower + assert "moderate" in sql_lower + assert "reported_month" in sql_lower + assert "2024-01" in sql_lower + + def test_ac1_no_filter_when_none( total_cost_binding: MetricBinding, damage_count_binding: MetricBinding,