feat(sql): generalize ClickHouse-builtin stub registration to ScalarUDF - #238
Merged
Conversation
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
force-pushed
the
feat/clickhouse-scalar-builtins-230
branch
from
August 22, 2026 21:11
22cbbdd to
8ba8fb8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
CLICKHOUSE_SCALAR_BUILTINStable tocrates/sql-function-catalog({ name, arity }, noRewriteKind-- see below) covering ten ClickHouse-only scalar builtins thebgp_jan2024_workloadcorpus (feat: add bgp_jan2024_workload SQL corpus test (200 queries, ASAPQuery PR #561) #219) surfaced as "unknown function"Planfailures:splitByChar,toDate,match,toStartOfHour,toStartOfWeek,toStartOfMinute,toStartOfFiveMinutes,toStartOfInterval,startsWith,positionCaseInsensitive.clickhouse_scalar_builtin_stub_udf(name, arity) -> ScalarUDFincrates/frontend-sql/src/sql/mod.rs, mirroringclickhouse_builtin_stub_udaf's "dead code by construction" reasoning: the stub'sinvoke/invoke_batchare unreachable because this front end only ever uses DataFusion for planning/type-checking, never physical execution.ScalarUDFper catalog entry inSqlLowerer::build_context(), alongside the existing per-entryAggregateUDFloop.FunctionRewriteis added for these, unlike the aggregate case (SQL: extract dialect builtin-function catalog into independent, generatable data (inspired by polyglot-sql-function-catalogs) #225/feat(sql): extract function-name catalog, generalize uniqExact/countIf rewrite #227):expr.rs'sExpr::ScalarFunctionarm already lowers any scalar call generically toUnresolved::FunctionCall { name, args }, so teaching DataFusion's planner to accept the name is the entire fix.Arities and return types (for sanity-checking against real ClickHouse semantics)
splitByChar(sep, s[, max_substrings])List<Utf8>Array(String); a plainUtf8element list is close enough for planning.toDate(expr)Date32toDate(timestamp), always 1 arg).match(haystack, pattern)BooleanUInt8(0/1), but every corpus use is aWHERE/boolean-context predicate.toStartOfHour(dt[, tz])Timestamp(Millisecond, None)Timestampconventiontypes.rs::dtype_to_arrowalready uses.toStartOfWeek(dt[, mode[, tz]])Timestamp(Millisecond, None)mode+timezoneargs.toStartOfMinute(dt[, tz])Timestamp(Millisecond, None)toStartOfFiveMinutes(dt[, tz])Timestamp(Millisecond, None)toStartOfInterval(dt, INTERVAL x unit[, tz])Timestamp(Millisecond, None)INTERVAL x unitclause parses as a single expr argument.startsWith(s, prefix)BooleanUInt8; treated as a predicate here.positionCaseInsensitive(haystack, needle[, start_pos])UInt64Arities were checked against actual corpus call sites (
grep'd out ofbgp_jan2024_rrc00_200_query_workload.yaml) as well as ClickHouse's documented signatures, so the stubSignatureaccepts 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):LoweredPlanNotImplementedUnsupportedFeatureOtherSchema/ParseThe
+2inNotImplementedis 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-indexNotImplementedgap instead of failing earlier at "unknown function".The
+2inOtheris a new, separate pre-existing gap this fix surfaces rather than causes: twotoStartOfInterval(timestamp, INTERVAL 15 minute)-shaped queries now plan far enough thattypes::scalar_value_to_asaphas to convert theINTERVALliteral (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'stoStartOfInterval(...)call itself now plans fine, so planning proceeds into its nestedtoIntervalMinute(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)
lagInFrame(window function, SQL: ClickHouse's lagInFrame window function needs window-frame modeling #231) andargMax(new aggregate semantic, SQL: ClickHouse's argMax has no AggIntent representation #232).NotImplementedgap and theINTERVAL-literalOthergap noted above.Test plan
cargo build --workspace --all-targetscargo test --workspace(all green)cargo fmt --all -- --checkcargo clippy --workspace --all-targets --all-features -- -D warningsCloses #230