[feature](lance) push down common string and boolean predicates - #67051
[feature](lance) push down common string and boolean predicates#67051Jay-ju wants to merge 2 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
1 similar comment
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review outcome: request changes.
I verified the authoritative bundle against the live PR head 597d3fd and base 0e06fc1, reviewed all four changed files, and traced the relevant function-resolution, Nereids translation, scan residual/limit, Doris LIKE, and exact lance-c/DataFusion/Arrow paths.
Two correctness blockers remain: name-only conversion can substitute a Lance built-in for a resolved same-named Doris UDF, and embedded-NUL wildcard LIKE patterns can be pushed despite a proven Doris-versus-Arrow semantic mismatch. Both pushed conjuncts are removed from Doris residual evaluation, so either case can return wrong rows and may also make limit pushdown eligible.
Critical checkpoints: ordinary LIKE/NOT LIKE/ESCAPE behavior, nullable-boolean three-valued logic, recursive boolean forms, LargeUtf8 variation, and the exact native consumer stack were checked with no additional defect found. The added unit/regression coverage exercises the normal paths but does not fence the two boundary cases above. The bundle and live PR had no existing inline or raw review comments, so these are not duplicates. Current lightweight CI, dependency review, style, license, and title checks pass; BE UT on macOS and the automated code-review check are still pending. Per the review contract, I did not run builds or tests locally.
User focus: no additional focus was provided. A fresh Round 2 implementation, tests/integration, and risk-boundary audit all returned NO_NEW_VALUABLE_FINDINGS against this exact two-comment set.
| if (function.getFnName() == null || function.getChildren().size() != 2) { | ||
| return Optional.empty(); | ||
| } | ||
| String functionName = function.getFnName().getFunction().toLowerCase(Locale.ROOT); |
There was a problem hiding this comment.
Preserve the resolved function identity before dispatching by name. FunctionRegistry intentionally allows a UDF to shadow a built-in via prefer_udf_over_builtin, while a qualified call selects the UDF directly, and Nereids translates Java/Python UDFs to this same FunctionCallExpr with a non-BUILTIN catalog function. A two-argument UDF named starts_with, ends_with, or like can therefore reach this switch, be replaced by Lance's built-in, and be removed from the Doris residual in LanceScanNode; an empty residual can also enable limit pushdown. Please require a non-null resolved Doris BUILTIN before the switch (leaving unknown/UDF identities residual) and cover an analyzed same-named UDF.
There was a problem hiding this comment.
Fixed in f613605. FunctionCallExpr pushdown now requires a resolved Doris BUILTIN function; unresolved functions and same-named UDFs remain as Doris residuals. Added a unit case with a resolved JAVA_UDF named starts_with.
| String patternValue = literal.getStringValue(); | ||
| // Doris uses backslash as LIKE's default escape character, while the Substrait function | ||
| // has no escape argument. Keep escaped LIKE patterns in Doris rather than changing meaning. | ||
| if (rejectEscapedPattern && patternValue.indexOf('\\') >= 0) { |
There was a problem hiding this comment.
Keep embedded-NUL LIKE patterns residual. In the default SQL mode Doris decodes backslash-zero before this guard; a wildcard pattern then reaches hs_compile through re_pattern.c_str(), so the regex is truncated at NUL (for example, a_ becomes ^a and matches abc). The exact lance-c 0.1.6 / DataFusion 53.1.0 / Arrow 58.3.0 consumer preserves the full NUL-bearing pattern, so pushdown filters abc out and the removed Doris residual cannot restore it. Please reject NUL here and add an analyzed end-to-end residual/result case.
There was a problem hiding this comment.
Fixed in f613605. String predicates containing an embedded NUL now remain residual. Added unit coverage for both LikePredicate and FunctionCallExpr forms, plus an end-to-end regression query that checks the residual plan and Doris result.
|
https://github.com/apache/doris-website/blob/master/versioned_docs/version-4.x/lakehouse/catalogs/lance-catalog.mdx |
|
/review |
|
Documentation update opened: apache/doris-website#4095. It covers direct Boolean predicates, LIKE/NOT LIKE, starts_with/ends_with, pushdown requirements, and the residual cases for escaped/NUL patterns and same-named UDFs in both English and Chinese 4.x docs. |
|
run buildall |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
4 participants
What problem does this PR solve?
Lance regular scans currently leave common string predicates and direct boolean predicates in Doris, so Lance cannot filter rows before Arrow materialization and transfer.
What is changed?
LIKE/NOT LIKEpredicates without backslash escapes through the existing Substrait filter.starts_with(column, literal)andends_with(column, literal).NOT boolean_column.Testing
mvn -pl fe-core -am -Dtest=LancePredicateConverterTest -DfailIfNoTests=false test(21 passed)test_lance_scalar_predicate_pushdownagainst local MinIO + FE + BEstarts_withverificationRelease note
Improve Lance predicate pushdown for common string and boolean expressions.