Skip to content

Add clickhouse_push_fail() for params, dictGet#34

Merged
theory merged 1 commit into
mainfrom
lookup-function
Nov 10, 2025
Merged

Add clickhouse_push_fail() for params, dictGet#34
theory merged 1 commit into
mainfrom
lookup-function

Conversation

@theory
Copy link
Copy Markdown
Collaborator

@theory theory commented Nov 9, 2025

Add a new function clickhouse_push_fail(), that just looks up the nam of the SQL function and raises an exception reporting that it failed to be pushed down. Use it for params(), which previously would crash the server because it wasn't passing its function name to clickhouse_func_push_fail().

Also use clickhouse_push_fail() for dictGet(), and remove the now unused function ch_push_func_text(). The only side effect is that clickhouse_push_fail() reports PostgreSQL canonical function names, which is to say all lowercase, e.g., dictget() instead of dictGet().

To better represent that it reports pushdown failures for any operation passed as the first argument, rename clickhouse_func_push_fail() to clickhouse_op_push_fail().

While at it, test the pushdown failure of params() in test/sql/function_pushdown.sql; this call previously would crash the server. Also test the pushdown failure of quantile() and quantileExact().

@theory theory requested a review from serprex November 9, 2025 21:59
@theory theory self-assigned this Nov 9, 2025
@theory theory force-pushed the lookup-function branch 2 times, most recently from 92576be to 19a5f81 Compare November 9, 2025 22:27
Add a new function `clickhouse_push_fail()`, that just looks up the name
of the SQL function and raises an exception reporting that it failed to
be pushed down. Use it for `params()`, which previously would crash the
server because it wasn't passing its function name to
`clickhouse_func_push_fail()`.

Also use `clickhouse_push_fail()` for `dictGet()`, and remove the now
unused function `ch_push_func_text()`. The only side effect is that
`clickhouse_push_fail()` reports PostgreSQL canonical function names,
which is to say all lowercase, e.g., `dictget()` instead of `dictGet()`.

To better represent that it reports pushdown failures for any operation
passed as the first argument, rename `clickhouse_func_push_fail()` to
`clickhouse_op_push_fail()`.

While at it, test the pushdown failure of `params()` in
`test/sql/function_pushdown.sql`; this call previously would crash the
server. Also test the pushdown failure of `quantile()` and
`quantileExact()`.
@theory theory merged commit 0a5d31b into main Nov 10, 2025
32 checks passed
@theory theory deleted the lookup-function branch November 10, 2025 17:55
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.

2 participants