Skip to content

feat(sql): generalize ClickHouse-builtin stub registration to ScalarUDF - #238

Merged
zzylol merged 1 commit into
mainfrom
feat/clickhouse-scalar-builtins-230
Aug 22, 2026
Merged

feat(sql): generalize ClickHouse-builtin stub registration to ScalarUDF#238
zzylol merged 1 commit into
mainfrom
feat/clickhouse-scalar-builtins-230

Conversation

@zzylol

@zzylol zzylol commented Aug 22, 2026

Copy link
Copy Markdown
Contributor

Summary

Arities and return types (for sanity-checking against real ClickHouse semantics)

Function Arity Arrow return type Why
splitByChar(sep, s[, max_substrings]) 2-3 List<Utf8> Real return is Array(String); a plain Utf8 element list is close enough for planning.
toDate(expr) 1 Date32 Matches actual corpus usage (toDate(timestamp), always 1 arg).
match(haystack, pattern) 2 Boolean ClickHouse returns UInt8 (0/1), but every corpus use is a WHERE/boolean-context predicate.
toStartOfHour(dt[, tz]) 1-2 Timestamp(Millisecond, None) Matches the Timestamp convention types.rs::dtype_to_arrow already uses.
toStartOfWeek(dt[, mode[, tz]]) 1-3 Timestamp(Millisecond, None) ClickHouse's optional mode + timezone args.
toStartOfMinute(dt[, tz]) 1-2 Timestamp(Millisecond, None)
toStartOfFiveMinutes(dt[, tz]) 1-2 Timestamp(Millisecond, None)
toStartOfInterval(dt, INTERVAL x unit[, tz]) 2-3 Timestamp(Millisecond, None) The INTERVAL x unit clause parses as a single expr argument.
startsWith(s, prefix) 2 Boolean ClickHouse returns UInt8; treated as a predicate here.
positionCaseInsensitive(haystack, needle[, start_pos]) 2-3 UInt64 1-based match position, 0 if not found.

Arities were checked against actual corpus call sites (grep'd out of bgp_jan2024_rrc00_200_query_workload.yaml) as well as ClickHouse's documented signatures, so the stub Signature accepts every real call shape in the corpus plus documented optional trailing args (timezone, mode, max-substrings, start-position).

Corpus tally shift

bgp_jan2024_workload's pinned aggregate tally (corpus_lowering_matches_the_pinned_aggregate_tally):

Category Before After
Lowered 97 145
Plan 92 40
NotImplemented 5 7
UnsupportedFeature 6 6
Other 0 2
Schema / Parse 0 / 0 0 / 0

The +2 in NotImplemented is exactly the companion gap the issue itself flags: splitByChar(...)[-1]-style calls (and a couple of other array/map-index uses) now plan far enough to hit the pre-existing map/array-index NotImplemented gap instead of failing earlier at "unknown function".

The +2 in Other is a new, separate pre-existing gap this fix surfaces rather than causes: two toStartOfInterval(timestamp, INTERVAL 15 minute)-shaped queries now plan far enough that types::scalar_value_to_asap has to convert the INTERVAL literal (DfScalarValue::IntervalMonthDayNano), which it doesn't support yet -- out of scope for this issue (which is about function names, not literal-expression conversion), so it's pinned rather than chased down, the same way the array-index gap is.

Also updates bgp_analytics's pinned per-query outcome table: query 7's toStartOfInterval(...) call itself now plans fine, so planning proceeds into its nested toIntervalMinute(5) argument -- an unregistered ClickHouse builtin outside this issue's 10-function scope -- and fails there instead (Expected::UnknownFunction("tostartofinterval") -> Expected::UnknownFunction("tointervalminute")).

Explicitly out of scope (per the issue)

Test plan

  • cargo build --workspace --all-targets
  • cargo test --workspace (all green)
  • cargo fmt --all -- --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings

Closes #230

Adds a CLICKHOUSE_SCALAR_BUILTINS catalog table (name + arity, no
RewriteKind needed) and clickhouse_scalar_builtin_stub_udf, mirroring
clickhouse_builtin_stub_udaf but for scalar functions -- registered in
SqlLowerer::build_context() alongside the existing AggregateUDF loop.
Covers splitByChar, toDate, match, the toStartOf* family, startsWith,
and positionCaseInsensitive, all currently "unknown function" in the
bgp_jan2024_workload corpus.

Unlike the aggregate case, no FunctionRewrite is needed: expr.rs's
Expr::ScalarFunction arm already lowers any scalar call generically to
Unresolved::FunctionCall { name, args }, so teaching DataFusion's
planner to accept the name is the whole fix.

bgp_jan2024_workload's pinned tally shifts from {Lowered: 97, Plan: 92,
NotImplemented: 5, UnsupportedFeature: 6} to {Lowered: 145, Plan: 40,
NotImplemented: 7, UnsupportedFeature: 6, Other: 2} -- the two new
Other-category queries hit a separate, pre-existing gap (an INTERVAL
literal that types::scalar_value_to_asap doesn't convert yet), surfaced
now that toStartOfInterval's name itself is accepted.

bgp_analytics' pinned per-query outcome also updates: query 7's
toStartOfInterval(...) is now accepted, so planning proceeds into its
nested toIntervalMinute(5) argument -- an unregistered builtin outside
this issue's 10-function scope -- and fails there instead.

Closes #230

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@zzylol
zzylol force-pushed the feat/clickhouse-scalar-builtins-230 branch from 22cbbdd to 8ba8fb8 Compare August 22, 2026 21:11
@zzylol
zzylol merged commit 3a2c6f4 into main Aug 22, 2026
4 of 5 checks passed
@zzylol
zzylol deleted the feat/clickhouse-scalar-builtins-230 branch August 22, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SQL: generalize the ClickHouse-builtin stub-registration mechanism from AggregateUDF to ScalarUDF

1 participant