diff --git a/.gitignore b/.gitignore index 4973f2fa..7333e1f2 100644 --- a/.gitignore +++ b/.gitignore @@ -5,3 +5,7 @@ # Regenerated by tools/dag-viewer/generate-sample.sh; dag.example.json is the committed sample /tools/dag-viewer/dag.json + +# Python bytecode cache (tools/clickhouse/extract_functions.py) +__pycache__/ +*.pyc diff --git a/crates/frontend-sql/src/sql/mod.rs b/crates/frontend-sql/src/sql/mod.rs index 161618bc..b545d262 100644 --- a/crates/frontend-sql/src/sql/mod.rs +++ b/crates/frontend-sql/src/sql/mod.rs @@ -1481,3 +1481,87 @@ fn lower_window_func_kind(fun: &WindowFunctionDefinition) -> Result = state + .aggregate_functions() + .keys() + .map(String::as_str) + .filter(|name| { + asap_sql_function_catalog::lookup_native(name).is_none() + && asap_sql_function_catalog::lookup_clickhouse_builtin(name).is_none() + && !asap_sql_function_catalog::KNOWN_UNMAPPED_NATIVE_FUNCTIONS.contains(name) + }) + .collect(); + uncovered.sort_unstable(); + assert!( + uncovered.is_empty(), + "DataFusion resolves these aggregate names but the catalog doesn't know about them \ + (crates/sql-function-catalog/src/lib.rs): {uncovered:?}\n\ + Either add a `NativeFunction` entry mapping each to its `AggSemantic`, or -- if it's \ + a deliberate non-goal (no `AggIntent` shape for it, or it's rejected elsewhere) -- \ + add it to `KNOWN_UNMAPPED_NATIVE_FUNCTIONS` with a reason. This usually means a \ + DataFusion version bump added or renamed a builtin aggregate." + ); + } + + /// Every `KNOWN_UNMAPPED_NATIVE_FUNCTIONS` entry earns its place by + /// actually being a name DataFusion resolves today — otherwise it is + /// stale documentation for a name that no longer exists (e.g. a prior + /// DataFusion version renamed it), not a real "deliberately not mapped" + /// decision, and should be removed. + #[test] + fn known_unmapped_entries_are_all_real_datafusion_names() { + let catalog = SqlCatalog::new(); + let ctx = SqlLowerer::new(&catalog) + .build_context() + .expect("build_context with an empty table catalog cannot fail"); + let resolved = ctx.state().aggregate_functions().clone(); + for name in asap_sql_function_catalog::KNOWN_UNMAPPED_NATIVE_FUNCTIONS { + assert!( + resolved.contains_key(*name), + "`{name}` is listed in KNOWN_UNMAPPED_NATIVE_FUNCTIONS but DataFusion no longer \ + resolves it -- remove the stale entry" + ); + } + } +} diff --git a/crates/sql-function-catalog/src/lib.rs b/crates/sql-function-catalog/src/lib.rs index c60cc52d..cff89361 100644 --- a/crates/sql-function-catalog/src/lib.rs +++ b/crates/sql-function-catalog/src/lib.rs @@ -254,6 +254,78 @@ pub const CLICKHOUSE_BUILTINS: &[ClickHouseBuiltin] = &[ }, ]; +/// Aggregate function names DataFusion's own planner resolves out of the box +/// that this catalog deliberately does *not* map to a canonical +/// [`AggSemantic`] -- either because `AggIntent` has no shape for them +/// (a multi-column correlation/regression aggregate, a bitwise/boolean +/// aggregate, a string concatenation aggregate, ...) or because this front +/// end already rejects them explicitly elsewhere (`array_agg`, `grouping`). +/// +/// This list exists for one reason: `asap-frontend-sql`'s DataFusion-registry +/// drift test (issue #225, item 3 -- see +/// `asap_frontend_sql::sql::catalog_drift`) walks a real `SessionContext`'s +/// resolved aggregate names and requires every one to be either in +/// [`NATIVE_FUNCTIONS`], a [`CLICKHOUSE_BUILTINS`] name, or listed here. A +/// name landing here is a *recorded decision*, not a silenced test failure -- +/// each group below says why. Do not add an entry just to make the test +/// pass; add a `NativeFunction` instead if the name should actually lower. +/// +/// Two known, narrow gaps rather than a deliberate non-goal: `var_sample` and +/// `var_population` are DataFusion's own alias spellings of `var_samp` / +/// `var_pop` (see `variance.rs`'s `aliases()` in `datafusion-functions- +/// aggregate`) that [`NATIVE_FUNCTIONS`] doesn't also list under those +/// spellings. Surfaced here rather than silently added to +/// [`NATIVE_FUNCTIONS`], since accepting a new spelling is a maintainer's +/// call, not something this catalog should do on its own. +pub const KNOWN_UNMAPPED_NATIVE_FUNCTIONS: &[&str] = &[ + // Selector aggregates: this front end only supports these as window + // functions (`WindowFuncKind::FirstValue`/`LastValue`/`NthValue`, via + // `OVER (...)`), not as plain `GROUP BY` aggregates -- no `AggIntent` + // variant models "the value from a particular row" as a reduction. + "first_value", + "last_value", + "nth_value", + // Bitwise / boolean aggregates -- no corresponding `AggIntent` variant. + "bit_and", + "bit_or", + "bit_xor", + "bool_and", + "bool_or", + // Two-column correlation / linear-regression aggregates -- every + // `AggIntent` value reducer takes one input column (`reducer_col` in + // `asap-frontend-sql`), so these have no home yet. + "corr", + "covar", + "covar_pop", + "covar_samp", + "regr_avgx", + "regr_avgy", + "regr_count", + "regr_intercept", + "regr_r2", + "regr_slope", + "regr_sxx", + "regr_sxy", + "regr_syy", + // String concatenation -- no `AggIntent` equivalent. + "string_agg", + // The weighted-percentile variant of `approx_percentile_cont` (an extra + // weight-column argument); only the unweighted form is in + // `NATIVE_FUNCTIONS`. + "approx_percentile_cont_with_weight", + // Explicitly rejected elsewhere, not merely unmapped: + // `array_agg_is_deliberately_rejected` (asap-frontend-sql's + // sql_lowering tests) covers `array_agg`; `lower_grouping_sets`'s own + // doc comment covers `grouping` (`GROUPING(col)` -- observable only via + // the `__grouping_id` discriminator this front end drops). + "array_agg", + "grouping", + // Known narrow gaps (see doc comment above) -- alias spellings of + // `var_samp` / `var_pop` this catalog doesn't accept yet. + "var_sample", + "var_population", +]; + /// Look up a native function name's canonical semantic (case-sensitive -- /// callers normalize case first, as `asap-frontend-sql` already does via /// `.to_lowercase()`). @@ -316,4 +388,38 @@ mod tests { ); } } + + /// `KNOWN_UNMAPPED_NATIVE_FUNCTIONS` documents names this catalog + /// deliberately does *not* map -- it should never overlap with a table + /// that *does* map the same name (that would be a contradiction: mapped + /// and "known unmapped" at once), and shouldn't repeat itself either + /// (each name is a one-time recorded decision). + #[test] + fn known_unmapped_list_is_disjoint_from_the_mapped_tables_and_has_no_duplicates() { + for name in KNOWN_UNMAPPED_NATIVE_FUNCTIONS { + assert!( + lookup_native(name).is_none(), + "{name} is both in NATIVE_FUNCTIONS and KNOWN_UNMAPPED_NATIVE_FUNCTIONS" + ); + assert!( + lookup_clickhouse_builtin(name).is_none(), + "{name} is both in CLICKHOUSE_BUILTINS and KNOWN_UNMAPPED_NATIVE_FUNCTIONS" + ); + } + let mut sorted = KNOWN_UNMAPPED_NATIVE_FUNCTIONS.to_vec(); + sorted.sort_unstable(); + sorted.dedup(); + assert_eq!( + sorted.len(), + KNOWN_UNMAPPED_NATIVE_FUNCTIONS.len(), + "KNOWN_UNMAPPED_NATIVE_FUNCTIONS has a duplicate entry" + ); + } + + #[test] + fn known_unmapped_names_are_lowercase() { + for name in KNOWN_UNMAPPED_NATIVE_FUNCTIONS { + assert_eq!(*name, name.to_lowercase(), "not lowercase: {name}"); + } + } } diff --git a/tools/clickhouse/README.md b/tools/clickhouse/README.md new file mode 100644 index 00000000..0bd59a4c --- /dev/null +++ b/tools/clickhouse/README.md @@ -0,0 +1,62 @@ +# ClickHouse function-catalog extraction (issue #225, item 3) + +`extract_functions.py` is a **dev-only, manual discovery tool**. It compares +ClickHouse's own aggregate-function list (`system.functions`) against +[`asap_sql_function_catalog::CLICKHOUSE_BUILTINS`](../../crates/sql-function-catalog/src/lib.rs) +and reports: + +- **candidates** -- ClickHouse aggregate names not yet in `CLICKHOUSE_BUILTINS` + (things worth considering adding), +- **possibly stale** -- `CLICKHOUSE_BUILTINS` entries that no longer appear in + ClickHouse's own list (e.g. renamed or removed upstream), and +- **combinator-derived** -- catalog entries like `countif` that are really a + base function (`count`) plus one of ClickHouse's + [aggregate combinators](https://clickhouse.com/docs/en/sql-reference/aggregate-functions/combinators) + (`-If`, `-Distinct`, `-Array`, ...), which ClickHouse doesn't enumerate as + its own `system.functions` row -- reported separately so it isn't + misreported as stale. + +It **only reports**. It never edits `crates/sql-function-catalog/src/lib.rs` -- +existence (+ arity) is all issue #225 asks the catalog to track, and deciding +each new entry's `RewriteKind` / canonical semantic is a human judgment call, +not something this script attempts. + +## Why this isn't wired into CI + +ClickHouse's function surface can only be introspected against an actual +ClickHouse (`system.functions` isn't documented data, it's a live catalog +table). This repo has no ClickHouse service anywhere in CI, and the project +decided deliberately not to add one just for this — see issue #225's own +"needs a decision on where such extraction tooling would actually run" open +question. So: a maintainer runs this locally, by hand, whenever they want to +check for drift or discover new candidates. It's never invoked automatically. + +## Running it + +This tool uses [`chdb`](https://github.com/chdb-io/chdb) -- ClickHouse +embedded as an in-process Python library. There's no server to start: + +```sh +pip install chdb +python3 tools/clickhouse/extract_functions.py +``` + +Optional flags: + +- `--repo-root PATH` -- repo root containing `crates/sql-function-catalog` + (default: inferred from this script's own location). +- `--output PATH` -- write the report to a file instead of stdout. + +Because `chdb` runs ClickHouse in-process rather than connecting to a +server, the report reflects whatever ClickHouse version the installed +`chdb` package embeds. `pip install --upgrade chdb` to check against a +newer ClickHouse release. + +## Running its tests + +The catalog-parsing and diff logic (everything except the actual `chdb` +query) has plain `unittest` coverage that needs no ClickHouse at all: + +```sh +cd tools/clickhouse && python3 -m unittest test_extract_functions -v +``` diff --git a/tools/clickhouse/extract_functions.py b/tools/clickhouse/extract_functions.py new file mode 100755 index 00000000..4dcbe99f --- /dev/null +++ b/tools/clickhouse/extract_functions.py @@ -0,0 +1,230 @@ +#!/usr/bin/env python3 +"""Dev-only discovery tool for issue #225, item 3. + +Compares ClickHouse's own aggregate-function surface (`system.functions`) +against `asap_sql_function_catalog::CLICKHOUSE_BUILTINS` +(`crates/sql-function-catalog/src/lib.rs`) and reports the diff: + + * candidates -- ClickHouse aggregate names not yet in `CLICKHOUSE_BUILTINS` + (things a maintainer might want to add support for) + * possibly stale -- `CLICKHOUSE_BUILTINS` entries that no longer appear in + ClickHouse's own list (e.g. renamed or removed upstream) + +This is a *reporting* tool for a human to act on, not a code generator: it +never touches `crates/sql-function-catalog/src/lib.rs`. Existence (+ arity, +which ClickHouse's `system.functions` doesn't usefully expose per-overload) +is all the issue's own scope asks for -- deciding each new entry's +`RewriteKind`/semantic is still a judgment call for whoever reads the report. + +Not wired into CI (see `tools/clickhouse/README.md`): this repo has no +ClickHouse service anywhere in its CI, and the project decided to keep it +that way. Run it locally instead. + +Uses `chdb` (https://github.com/chdb-io/chdb) -- ClickHouse embedded as an +in-process Python library -- so there's no server to start: `pip install +chdb` is the only setup step. `chdb` is only imported inside +`fetch_clickhouse_aggregate_names`, so every other function here (the +catalog-source parsing, the diffing, the report formatting) runs and is +unit-testable without it installed. +""" + +from __future__ import annotations + +import argparse +import json +import re +import sys +from pathlib import Path + +# Relative to the repo root; matches the crate PR #227 introduced. +CATALOG_SRC_PATH = "crates/sql-function-catalog/src/lib.rs" + +# ClickHouse's aggregate-combinator suffixes (-If, -Array, -Distinct, ...): +# https://clickhouse.com/docs/en/sql-reference/aggregate-functions/combinators +# apply to *any* base aggregate function and are never enumerated as their +# own `system.functions` row (there is no row named "countIf" -- only +# "count"). A `CLICKHOUSE_BUILTINS` entry like "countif" is therefore +# base name "count" + combinator "If", not a name ClickHouse lists on its +# own -- so a naive diff would misreport it as stale every single run. These +# are ordered longest-suffix-first so "-OrDefault"/"-OrNull" aren't shadowed +# by a shorter accidental match. +KNOWN_COMBINATOR_SUFFIXES = ( + "ordefault", + "ornull", + "distinct", + "resample", + "simplestate", + "foreach", + "state", + "merge", + "array", + "if", +) + + +def parse_catalog_names(source: str) -> set[str]: + """Extract every `name: "..."` inside the `CLICKHOUSE_BUILTINS` const + array from `source` (the text of sql-function-catalog's `lib.rs`). + + Deliberately a small regex over the block, not a Rust parser -- the + catalog's own `name` fields are plain string literals, and slicing out + just the `CLICKHOUSE_BUILTINS` block first (rather than matching + `name: "..."` over the whole file) keeps this from also picking up + `NativeFunction` entries, which are a different table with a different + meaning here. + """ + start = source.find("pub const CLICKHOUSE_BUILTINS") + if start == -1: + raise ValueError( + f"couldn't find `pub const CLICKHOUSE_BUILTINS` in {CATALOG_SRC_PATH} -- " + "has it been renamed or moved?" + ) + end = source.find("\n];", start) + if end == -1: + raise ValueError( + "found `CLICKHOUSE_BUILTINS` but not its closing `];` -- " + "malformed source or the array's formatting changed" + ) + block = source[start:end] + return set(re.findall(r'name:\s*"([a-z0-9_]+)"', block)) + + +def load_catalog_names(repo_root: Path) -> set[str]: + path = repo_root / CATALOG_SRC_PATH + return parse_catalog_names(path.read_text()) + + +def fetch_clickhouse_aggregate_names() -> set[str]: + """Every aggregate function name ClickHouse's embedded `system.functions` + lists (lowercased -- `CLICKHOUSE_BUILTINS` names are lowercase, and SQL + function names are case-insensitive in ClickHouse anyway). Imports + `chdb` lazily so the rest of this module works without it installed. + """ + import chdb + + result = chdb.query( + "SELECT name FROM system.functions WHERE is_aggregate = 1 ORDER BY name FORMAT JSON", + "JSON", + ) + payload = json.loads(str(result)) + return {row["name"].lower() for row in payload["data"]} + + +def combinator_base(name: str) -> str | None: + """If `name` looks like `base + combinator suffix` (e.g. "countif" -> + "count"), return `base`. Otherwise `None`. Only used to explain an + apparently-stale catalog entry, never to invent new candidates -- a + bare suffix match is too weak a signal to report as "ClickHouse has + this function", only to note "this one isn't really missing". + """ + for suffix in KNOWN_COMBINATOR_SUFFIXES: + if name.endswith(suffix) and len(name) > len(suffix): + return name[: -len(suffix)] + return None + + +def diff( + catalog_names: set[str], clickhouse_names: set[str] +) -> tuple[list[str], list[str], list[tuple[str, str]]]: + """`(candidates, stale, combinator_derived)`. + + `candidates` -- ClickHouse aggregate names not in the catalog. + `stale` -- catalog names ClickHouse's own list doesn't contain *and* + that don't explain themselves as a combinator of a base ClickHouse + still lists (e.g. an actual rename/removal upstream). + `combinator_derived` -- catalog names ClickHouse's list doesn't contain + directly, but which are a combinator of a base name it does list + (e.g. ("countif", "count")) -- not stale, just not separately + enumerated; reported for visibility, not as an action item. + """ + candidates = sorted(clickhouse_names - catalog_names) + missing = catalog_names - clickhouse_names + stale: list[str] = [] + combinator_derived: list[tuple[str, str]] = [] + for name in sorted(missing): + base = combinator_base(name) + if base is not None and base in clickhouse_names: + combinator_derived.append((name, base)) + else: + stale.append(name) + return candidates, stale, combinator_derived + + +def format_report( + candidates: list[str], stale: list[str], combinator_derived: list[tuple[str, str]] +) -> str: + """Diff-friendly (sorted, one name per line) plain-text report.""" + lines: list[str] = [] + lines.append(f"# ClickHouse aggregate-function catalog diff ({CATALOG_SRC_PATH})") + lines.append("") + lines.append(f"## Candidates -- in ClickHouse, not in CLICKHOUSE_BUILTINS ({len(candidates)})") + lines.append("") + if candidates: + lines.extend(f" {name}" for name in candidates) + else: + lines.append(" (none)") + lines.append("") + lines.append(f"## Possibly stale -- in CLICKHOUSE_BUILTINS, not in ClickHouse ({len(stale)})") + lines.append("") + if stale: + lines.extend(f" {name}" for name in stale) + else: + lines.append(" (none)") + lines.append("") + lines.append( + "## Combinator-derived -- not separately enumerated by ClickHouse, " + f"not actually missing ({len(combinator_derived)})" + ) + lines.append("") + if combinator_derived: + lines.extend(f" {name} (= {base} + combinator)" for name, base in combinator_derived) + else: + lines.append(" (none)") + lines.append("") + return "\n".join(lines) + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument( + "--repo-root", + type=Path, + default=Path(__file__).resolve().parents[2], + help="repo root containing crates/sql-function-catalog (default: inferred from this file's location)", + ) + parser.add_argument( + "--output", + type=Path, + default=None, + help="write the report to this file instead of stdout", + ) + args = parser.parse_args(argv) + + try: + catalog_names = load_catalog_names(args.repo_root) + except (OSError, ValueError) as e: + print(f"error: {e}", file=sys.stderr) + return 1 + + try: + clickhouse_names = fetch_clickhouse_aggregate_names() + except ImportError: + print( + "error: chdb is not installed -- run `pip install chdb` first " + "(see tools/clickhouse/README.md)", + file=sys.stderr, + ) + return 1 + + candidates, stale, combinator_derived = diff(catalog_names, clickhouse_names) + report = format_report(candidates, stale, combinator_derived) + + if args.output: + args.output.write_text(report) + else: + print(report) + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/tools/clickhouse/test_extract_functions.py b/tools/clickhouse/test_extract_functions.py new file mode 100644 index 00000000..b70ecb1a --- /dev/null +++ b/tools/clickhouse/test_extract_functions.py @@ -0,0 +1,160 @@ +"""Unit tests for `extract_functions.py`'s catalog-parsing and diff logic -- +deliberately exercised without a live (or embedded) ClickHouse: `chdb` is +never imported here, matching `extract_functions.fetch_clickhouse_aggregate_names` +being the only function in that module that needs it. + +Run with: python3 -m unittest tools/clickhouse/test_extract_functions.py +(or `python3 -m pytest tools/clickhouse/` if pytest is available -- these +are plain `unittest.TestCase`s, either runner works). Not wired into CI -- +see tools/clickhouse/README.md for why. +""" + +from __future__ import annotations + +import unittest + +from extract_functions import combinator_base, diff, format_report, parse_catalog_names + +# A trimmed stand-in for crates/sql-function-catalog/src/lib.rs's shape -- +# enough surrounding structure (a preceding NATIVE_FUNCTIONS-like table, the +# doc comment, the real field layout) to prove the parser targets the right +# block and ignores everything else, without depending on the real file's +# exact current contents (which is free to keep growing). +FIXTURE_SOURCE = ''' +pub const NATIVE_FUNCTIONS: &[NativeFunction] = &[ + NativeFunction { + name: "sum", + arity: Arity::Exact(1), + semantic: AggSemantic::Sum, + }, +]; + +/// ClickHouse-only builtin aggregate names DataFusion's planner has no +/// native equivalent for at all. +pub const CLICKHOUSE_BUILTINS: &[ClickHouseBuiltin] = &[ + ClickHouseBuiltin { + name: "uniqexact", + arity: Arity::Exact(1), + rewrite: RewriteKind::CountDistinct, + }, + ClickHouseBuiltin { + name: "countif", + arity: Arity::Exact(1), + rewrite: RewriteKind::CountIfToSum, + }, +]; + +pub fn lookup_native(name: &str) -> Option { + NATIVE_FUNCTIONS.iter().find(|f| f.name == name).map(|f| f.semantic) +} +''' + + +class ParseCatalogNamesTest(unittest.TestCase): + def test_extracts_every_name_in_the_clickhouse_builtins_block(self): + self.assertEqual(parse_catalog_names(FIXTURE_SOURCE), {"uniqexact", "countif"}) + + def test_does_not_pick_up_native_functions_entries(self): + names = parse_catalog_names(FIXTURE_SOURCE) + self.assertNotIn("sum", names) + + def test_raises_a_clear_error_when_the_const_is_missing(self): + with self.assertRaisesRegex(ValueError, "CLICKHOUSE_BUILTINS"): + parse_catalog_names("pub const NATIVE_FUNCTIONS: &[NativeFunction] = &[];") + + def test_raises_a_clear_error_when_the_closing_bracket_is_missing(self): + truncated = FIXTURE_SOURCE.split("pub const CLICKHOUSE_BUILTINS")[0] + ( + 'pub const CLICKHOUSE_BUILTINS: &[ClickHouseBuiltin] = &[\n' + ' ClickHouseBuiltin { name: "uniqexact", ' + ) + with self.assertRaisesRegex(ValueError, "closing"): + parse_catalog_names(truncated) + + +class CombinatorBaseTest(unittest.TestCase): + def test_recognizes_the_if_combinator(self): + self.assertEqual(combinator_base("countif"), "count") + + def test_recognizes_the_distinct_combinator(self): + self.assertEqual(combinator_base("sumdistinct"), "sum") + + def test_prefers_the_longer_suffix_over_a_shorter_one_it_contains(self): + # "ordefault" must not be misparsed as bare "if"/"or" matches. + self.assertEqual(combinator_base("sumordefault"), "sum") + + def test_returns_none_for_a_name_with_no_known_combinator_suffix(self): + self.assertIsNone(combinator_base("uniqexact")) + + def test_returns_none_rather_than_an_empty_base(self): + # A name that IS a bare suffix (no base in front) isn't a combinator + # of anything. + self.assertIsNone(combinator_base("if")) + + +class DiffTest(unittest.TestCase): + def test_candidates_are_clickhouse_names_the_catalog_lacks(self): + candidates, stale, combinator_derived = diff( + catalog_names={"uniqexact"}, + clickhouse_names={"uniqexact", "quantile", "median"}, + ) + self.assertEqual(candidates, ["median", "quantile"]) + self.assertEqual(stale, []) + self.assertEqual(combinator_derived, []) + + def test_a_removed_upstream_name_is_reported_stale(self): + candidates, stale, combinator_derived = diff( + catalog_names={"uniqexact", "somethingremoved"}, + clickhouse_names={"uniqexact"}, + ) + self.assertEqual(stale, ["somethingremoved"]) + self.assertEqual(combinator_derived, []) + + def test_a_combinator_of_a_still_present_base_is_not_reported_stale(self): + # "countif" isn't its own system.functions row -- it's "count" + the + # -If combinator -- so it must land in combinator_derived, not stale. + # "count" itself is a legitimate candidate here (the catalog only + # lists "countif", not the bare base) -- this test is only about + # `stale`/`combinator_derived`, so it isn't asserted on. + candidates, stale, combinator_derived = diff( + catalog_names={"countif"}, + clickhouse_names={"count"}, + ) + self.assertEqual(stale, []) + self.assertEqual(combinator_derived, [("countif", "count")]) + self.assertEqual(candidates, ["count"]) + + def test_a_combinator_whose_base_is_also_gone_is_reported_stale_not_combinator_derived(self): + # If the base itself vanished too, that's a real gap worth flagging, + # not quietly absorbed into "combinator, nothing to see here". + candidates, stale, combinator_derived = diff( + catalog_names={"countif"}, + clickhouse_names=set(), + ) + self.assertEqual(stale, ["countif"]) + self.assertEqual(combinator_derived, []) + + def test_empty_inputs_produce_empty_everything(self): + self.assertEqual(diff(set(), set()), ([], [], [])) + + +class FormatReportTest(unittest.TestCase): + def test_report_is_sorted_and_diff_friendly(self): + report = format_report(["a", "b"], ["z"], [("countif", "count")]) + lines = report.splitlines() + # Headers present in a stable order; a byte-for-byte snapshot would + # be too brittle across incidental wording tweaks, so this only + # pins the properties the tool's usefulness actually depends on. + self.assertIn("Candidates", report) + self.assertIn("Possibly stale", report) + self.assertIn("Combinator-derived", report) + self.assertLess(lines.index(" a"), lines.index(" b")) + self.assertIn(" z", report) + self.assertIn("countif", report) + + def test_empty_sections_say_so_rather_than_being_blank(self): + report = format_report([], [], []) + self.assertIn("(none)", report) + + +if __name__ == "__main__": + unittest.main()