morningman opened a new pull request, #70372:
URL: https://github.com/apache/doris/pull/70372
### Background
Connector plugins learn which filters they may push down from a
`ConnectorExpression` tree that fe-core's `ExprToConnectorExpressionConverter`
builds from a scan's conjuncts. A function call arrives as a
`ConnectorFunctionCall` with the function's name, return type and arguments,
and connectors that translate calls match it by name: JDBC writes it into the
remote SQL when `enable_ext_func_pred_pushdown` is on (the default) and the
name passes the database's rules (for MySQL, a blocklist of three names), and
Elasticsearch runs `esquery` as a native query.
### What problem does this PR solve?
Issue Number: None
Related PR: #70369 (changes the same converter and SPI package;
independent), #68702 (also changes the connector SPI; whichever merges second
takes the next plugin API major)
PR Type: Bugfix
Problem Summary:
A filter that calls a Java or Python UDF on a JDBC table is sent to the
remote database as a call of the same name. On MySQL, `WHERE mydb.my_udf(id) =
113` fails with `FUNCTION doris_test.my_udf does not exist`, and a UDF named
like a MySQL function, `mydb.abs(id) = 113`, is computed by MySQL instead,
which silently drops rows. The converter hands connectors only the function's
name, so no connector can tell a UDF from a builtin. This PR adds
`ConnectorFunctionCall#isBuiltin()`, set from the catalog function's binary
type; JDBC and Elasticsearch no longer translate a call that is not a builtin,
and Doris evaluates it.
### Details
- **Change map:** +373 −26 in 13 files, of which tests are +303: a
regression suite +89 with its `.out` +10, and unit tests +204. The production
code is +69 −23:
1. SPI (+42 −13): the flag on `ConnectorFunctionCall`, and Rule 6 of
`pushdown/package-info.java` rewritten; plus the plugin API major 30.0 → 31.0
in `fe/fe-connector/pom.xml`.
2. fe-core (+12 −4): `ExprToConnectorExpressionConverter` sets the flag.
3. Connectors (+15 −6): the checks in `JdbcQueryBuilder` and
`EsQueryDslBuilder`.
- **Frequency:** Always, with function pushdown on: any filter that calls a
UDF on a JDBC catalog over MySQL, and over ClickHouse or Oracle when the UDF's
name is on their allowlist or in `function_rules`. On Elasticsearch, a UDF
named `esquery`.
- **Root cause:** `ExprToConnectorExpressionConverter.convertFunctionCall`
keeps only `getFnName().getFunction()`, the return type and the arguments, and
drops `Function.BinaryType`. `JdbcQueryBuilder.functionCallToSql` then renders
any call that `JdbcFunctionPushdownConfig.canPushDown` accepts by name, and
`EsQueryDslBuilder` takes any call named `esquery` for the builtin. The SPI
documentation said calls with real arguments "are safe to match by name".
- **Why existing tests missed it:** `JdbcQueryBuilderTest` and
`EsScanPlanProviderTest` build calls by hand, all of them builtins, and no unit
test converts a UDF. The only regression suite that uses a UDF on a JDBC table,
`test_mysql_varbinary_with_udf`, calls it in a projection; no suite filters
with one.
- **Fix:** The converter sets the flag where the binary type is still known,
and each connector that matches calls by name checks it. JDBC refuses a
non-builtin call before rendering anything, so a fallback (an expression the
converter does not model, sent as SQL text) that calls a UDF stays local too; a
fallback is a builtin only when it calls no UDF. Elasticsearch requires the
builtin for `esquery`, and the converter's LIKE / REGEXP shortcut applies only
to builtins. The alternative, the engine keeping every conjunct with a UDF away
from all connectors as it does for CASTs, was not chosen: the flag leaves the
decision to each connector. Changing the SPI surface moves the plugin API major
to 31.0.
- **Other paths and branches:** Iceberg drops every function call, and Hive,
Hudi, Paimon, MaxCompute and Trino translate none by name, so they are not
affected. On branch-4.1 and branch-4.2 the legacy `JdbcScanNode` has the same
gap (`JdbcFunctionPushDownRule` checks names only); they have no connector SPI,
so this PR does not apply there.
- **Behavior change:** Such a filter no longer reaches the remote database,
so the remote query returns what the other conjuncts select, without LIMIT, and
Doris filters the rest. Connector plugins built against plugin API 30 are
refused by this FE until they are rebuilt.
### Testing
- New regression suite `external_table_p0/jdbc/test_jdbc_mysql_udf_filter`
(a Java UDF registered as `udf_add_one` and as `abs`, on MySQL 5.7): on the
unfixed engine its first check fails, MySQL receiving `WHERE (udf_add_one(`id`)
= 113)`; by hand on that engine, the query fails with MySQL error 1305, and
`mydb.abs(id) = 113` returns no row instead of 112. With the fix both return
112 with no WHERE in the remote query, and the builtin `abs` is still pushed.
Run locally against a standalone `mysql:5.7.36` container; the External
pipeline runs it (`enableJdbcTest`).
- Unit tests, locally: new `ExprToConnectorFunctionCallTest` (fe-core) and
`ConnectorFunctionCallTest` (SPI), and new cases in `JdbcQueryBuilderTest` and
`EsQueryDslBuilderTest`. All of fe-connector-spi (154), fe-connector-jdbc
(275), fe-connector-es (164), and in fe-core the converter tests with
`PluginDrivenScanNode*Test` (282), pass.
- Seven mutations, each failing a new test: the converter taking every call
for a builtin, the LIKE shortcut or the fallback ignoring the flag, `equals`
ignoring it, the 3-argument constructor meaning a UDF, and each connector check
removed.
### Notes for Reviewers
- The 3-argument `ConnectorFunctionCall` constructor still means a builtin;
its callers are the converter's arithmetic operators and connector tests.
- Rule 6's old example, `my_udf(a, b) = 1` arriving as one fallback, never
happened: a UDF call converts to a named call with arguments. The new example
is a CASE expression, which the converter does not model.
- `getFn()` is not null-checked: plugin scan conjuncts come from Nereids'
`ExpressionTranslator`, which always sets the catalog function.
- The Lance connector being ported from branch-4.2 (not yet opened) will
check the flag for `starts_with`, `ends_with`, `array_contains` and
`arrays_overlap`, as 4.2 checks the binary type.
### Release note
JDBC catalogs over MySQL, ClickHouse or Oracle no longer send a filter that
calls a Java or Python UDF to the remote database, where it failed with
"FUNCTION ... does not exist" or was computed by a remote function of the same
name; Doris evaluates it. On Elasticsearch catalogs, a UDF named esquery is no
longer run as an Elasticsearch query.
### Check List (For Author)
- Behavior changed: Yes (see Details)
- Does this need documentation: No
- Pre-submission status: Partially verified: the regression suite ran
against MySQL 5.7 only, not ClickHouse or Oracle; the other JDBC and
Elasticsearch suites were not run locally
### AI Disclosure
- Tool: Claude Code (Claude Opus 5.5)
- Generated: code, tests, this description
- Human review: The author reviewed part of the changes.
- Prompts: While porting the Lance pushdown from branch-4.2, the agent
reported that connectors cannot tell a Java UDF from a builtin of the same
name; the author chose to fix it at the SPI boundary in its own PR, with a flag
on the function call that each connector checks, and approved an end-to-end run
against a MySQL container.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
