Skip to content

[feature](lance) Support prepared vector search on branch-4.1 - #68162

Merged
yiguolei merged 3 commits into
apache:branch-4.1from
Gabriel39:dev/lance-prepared-statement-branch-4.1
Sep 20, 2026
Merged

yiguolei merged 3 commits into
apache:branch-4.1from
Gabriel39:dev/lance-prepared-statement-branch-4.1

Conversation

@Gabriel39

@Gabriel39 Gabriel39 commented Sep 18, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

vector_search("query_vector"=?, "top_k"=?) currently converts the placeholders into the literal string ?, so server-side prepared Lance vector queries fail during analysis.

Preserve placeholders for query_vector, top_k, offset, and filter, and bind fresh property values on every EXECUTE. PREPARE resolves the output schema without requiring a query vector. Table, column, and search configuration remain constant. Each execution performs normal analysis and planning and resolves Lance metadata again; the retained statement never caches a bound vector or snapshot.

This supports both indexed and flat searches and reuses the parsed statement. Forwarded executions preserve cached parameter types when the client omits the type table on repeated executions; the forwarding request includes the types needed by a fresh master-side PREPARE. It does not introduce an execution-plan cache or bypass the optimizer.

Example with MySQL Connector/J and useServerPrepStmts=true:

try (PreparedStatement statement = connection.prepareStatement(
        "SELECT id, _distance FROM vector_search("
        + "'table'='catalog.db.items', 'column'='embedding', "
        + "'query_vector'=?, 'top_k'=?, 'offset'=?, 'filter'=?)")) {
    for (String vector : vectors) {
        statement.setString(1, vector); // JSON array matching the vector column dimension.
        statement.setInt(2, 10);
        statement.setInt(3, 0);
        statement.setString(4, "id > 0");
        try (ResultSet rows = statement.executeQuery()) {
            // Consume the result before executing the next vector.
        }
    }
}

Release note

Support server-side prepared parameters for Lance vector_search query vectors, top-k, offsets, and filters.

Validation

  • Proxy PREPARE tests reproduce the stale planner context in both search modes before the fix; PREPARE now synchronizes the executor with the parser-created context before parameter decoding.
  • Parser and analyzer tests cover parameter retention, quoted question marks, case-insensitive properties, constant-only property rejection, repeated execution with different vectors and snapshots, mixed constants/parameters, and invalid or missing parameters.
  • A JDBC regression reuses one ServerPreparedStatement with A/B/A parameter changes and compares results with literal SQL for both indexed and flat searches. On live non-master FEs, it also prepares locally and forces repeated executions to the master.
  • A protocol test exercises five executions through the follower decoder, RPC request construction, Thrift serialization, and a fresh proxy decoder. It covers omitted type tables, unsigned values, NULL parameters across two bitmap bytes, and later parameter type changes. Local execution retains the original payload without copying it.
  • PREPARE and EXECUTE share single-vector and multi-vector schema validation. Tests cover schema-only PREPARE, multi-vector binding, and rejection of invalid dimensions and excessive candidate budgets at EXECUTE. The four CI FE UT failures caused by unbound-vector validation were reproduced before the fix.
  • Local verification: all 154 tests passed via run-fe-ut.sh across MysqlPreparedStatementForwardingTest, MysqlConnectProcessorCursorFetchTest, FEOpExecutorMysqlProtocolTest, LancePreparedSearchTest, LancePreparedStatementTest, LanceVectorQueryTest, VectorSearchTableValuedFunctionTest, FullTextSearchTableValuedFunctionTest, LanceScanNodeTest, PrepareTest, and NereidsParserTest. FE Checkstyle passed. The full external JDBC regression was also executed locally using the current PR FE build, a master FE, an observer FE, one BE, and the MinIO/Lance fixtures: 1 suite passed, 0 failures, 0 skips. It executed 12 real server-prepared queries covering direct and forwarded execution, indexed and flat search, and A/B/A parameter changes. The original script reproduced the CI String.positive() failure before fixing both JDBC URL concatenations.

Check List (For Author)

  • Test
    • Regression test
    • Unit Test
  • Behavior changed:
    • Yes. Supported vector_search property values can be prepared parameters.
  • Does this need documentation?
    • Yes. Usage and the supported parameter scope are documented above.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@Gabriel39
Gabriel39 requested a review from yiguolei as a code owner September 18, 2026 06:30
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review conclusion: request changes for one P1 correctness issue. A server-prepared SELECT with a parameterized vector_search property fails if it is prepared locally on a readable follower and a later execution is forwarded to the master: reconstructed PREPARE plans with the executor's unmarked StatementContext before the forwarded parameter packet is decoded. The planner context must be synchronized, and this path needs a forced-forward/proxy regression.

Critical checkpoints: the direct PREPARE/EXECUTE path preserves placeholder identity and rebuilds fresh bindings, authorization, Lance metadata, snapshot state, request values, and the global TopN on every execution. Schema-only PREPARE stops before physical translation, so its deferred vector and dummy values cannot reach a scan. Indexed and flat request construction, limit/offset handling, property validation, plan-copy behavior, cache fencing, and FE/BE request checks were traced without finding another defect. The added JDBC regression covers real server-prepared A/B/A reuse for both search modes, but it does not exercise execution forwarding. The expression AGENTS checkpoint is not implicated because no checkLegalityBeforeTypeCoercion overload was added or changed. No user-specific focus points were provided.

This was a static-only review as required; I did not run builds or tests. The PR author's reported FE tests/checkstyle and the fixture-gated external regression were not independently validated here. Two complete review rounds covered all nine changed files and converged on this single non-duplicate finding, with no unresolved candidates.

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of head 1364feb56e found one blocking correctness issue.

  • Prepared-property parsing, placeholder ID retention/copying, schema-only PREPARE deferral, and fresh per-EXECUTE vector/top-k/offset/filter/field-ID/snapshot rebuilding are internally consistent.
  • The prior forwarded PREPARE StatementContext issue is fixed at this head and was treated as an existing-thread duplicate fence.
  • Repeated forced forwarding still loses Connector/J's cached parameter types: the second same-typed COM_STMT_EXECUTE omits the type table while the master reconstructs an untyped PREPARE, so the A/B/A path fails before binding. See the inline P1.
  • The expressions review checkpoint is not directly implicated because no checkLegalityBeforeTypeCoercion overload changed.
  • No additional user focus was provided.

Validation status: static inspection only, as required by the runner contract. I did not run builds or tests. The author reports 107 FE tests and FE Checkstyle passed and the Groovy regression compiled; the external multi-FE Lance regression was not run, and static protocol tracing indicates its second forwarded iteration will fail.

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@Gabriel39

Copy link
Copy Markdown
Contributor Author

Fixed both JDBC URL concatenations in the Lance prepared-search regression. A leading + on a new line was parsed by Groovy as unary plus on a String, causing the runtime String.positive() error even though script compilation succeeded.

Verified locally before committing:

  • Built the current PR FE with build.sh --fe.
  • Ran the original complete suite and reproduced the same String.positive() failure.
  • Ran the fixed complete suite against a master FE, an observer FE, one BE, and MinIO/Lance fixtures: 1 suite passed, 0 failures, 0 skips.
  • All 12 actual ServerPreparedStatement executions passed, covering direct and forwarded execution, indexed and flat search, and repeated A/B/A parameter binding. Both connections retained emulateUnsupportedPstmts=false.

The fix does not weaken any assertions or skip forwarding coverage.

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Selected model is at capacity. Please try a different model.
Workflow run: https://github.com/apache/doris/actions/runs/35326619148

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Codex completed, but no new pull request review was submitted for the current head SHA.
Workflow run: https://github.com/apache/doris/actions/runs/35323527595

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 90.09% (100/111) 🎉
Increment coverage report
Complete coverage report

@Gabriel39
Gabriel39 force-pushed the dev/lance-prepared-statement-branch-4.1 branch from d8a1594 to cab91e1 Compare September 20, 2026 01:25
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review conclusion: no blocking issues found at head cab91e1. Summary-only approval comment; no inline comments.

Critical checkpoints:

  • Prepared vector_search properties are parsed with stable placeholder IDs, restricted to query_vector/top_k/offset/filter, reject dynamic names/duplicates/NULL or non-literal execute values, and are rebound into a fresh property map and VectorSearch/catalog function on every EXECUTE. Retained plans do not reuse a prior Lance vector, metadata snapshot, or catalog-function cache.
  • PREPARE defers only query-value-dependent vector encoding, shape/dimension, multi-vector metric, and candidate-budget checks. Table/catalog authorization, metadata/version, schema, field IDs, static options, filter encoding, and top-k/offset invariants remain enforced; PrepareCommandPlanner stops at analysis, so an unset query vector cannot reach physical scanning.
  • Repeated COM_STMT_EXECUTE forwarding preserves the follower's cached MySQL type codes and unsigned bits when the packet omits its type table, rebuilds the null bitmap/type table in little-endian form, copies values without mutating the client buffer, and retains zero-parameter forwarding behavior. The existing omitted-type forwarding thread is therefore a duplicate fence and is not resubmitted.
  • PrepareCommand now installs the parser-created prepare-stage StatementContext before slot planning and registration. The existing forwarded planner-context thread is fixed at this head and is also a duplicate fence.
  • The required expressions AGENTS checkpoint is not implicated: this PR adds no checkLegalityBeforeTypeCoercion overload or child traversal.
  • No user-specific review focus points were supplied.

The normal full review and the risk-focused forwarding/context review both returned NO_NEW_VALUABLE_FINDINGS after complete changed-file coverage and missed-issue rechecks. This was static-only review as required by the runner; I did not run builds or tests. The author-reported FE tests/checkstyle and fixture-gated external regression were not independently validated here. The repository contained no discoverable code-review SKILL.md, so the mandated review prompt and checked-in AGENTS guide were followed as the operative instructions.

@hello-stephen

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 89.91% (98/109) 🎉
Increment coverage report
Complete coverage report

@yiguolei
yiguolei merged commit 66ebcdd into apache:branch-4.1 Sep 20, 2026
31 of 32 checks passed
yiguolei pushed a commit that referenced this pull request Sep 21, 2026
### What problem does this PR solve?

`vector_search("query_vector"=?, "top_k"=?)` currently converts the
placeholders into the literal string `?`, so server-side prepared Lance
vector queries fail during analysis.

Preserve placeholders for `query_vector`, `top_k`, `offset`, and
`filter`, and bind fresh property values on every EXECUTE. PREPARE
resolves the output schema without requiring a query vector. Table,
column, and search configuration remain constant. Each execution
performs normal analysis and planning and resolves Lance metadata again;
the retained statement never caches a bound vector or snapshot.

This supports both indexed and flat searches and reuses the parsed
statement. Forwarded executions preserve cached parameter types when the
client omits the type table on repeated executions; the forwarding
request includes the types needed by a fresh master-side PREPARE. It
does not introduce an execution-plan cache or bypass the optimizer.

Example with MySQL Connector/J and `useServerPrepStmts=true`:

```java
try (PreparedStatement statement = connection.prepareStatement(
        "SELECT id, _distance FROM vector_search("
        + "'table'='catalog.db.items', 'column'='embedding', "
        + "'query_vector'=?, 'top_k'=?, 'offset'=?, 'filter'=?)")) {
    for (String vector : vectors) {
        statement.setString(1, vector); // JSON array matching the vector column dimension.
        statement.setInt(2, 10);
        statement.setInt(3, 0);
        statement.setString(4, "id > 0");
        try (ResultSet rows = statement.executeQuery()) {
            // Consume the result before executing the next vector.
        }
    }
}
```

### Release note

Support server-side prepared parameters for Lance `vector_search` query
vectors, top-k, offsets, and filters.

### Validation

- Proxy PREPARE tests reproduce the stale planner context in both search
modes before the fix; PREPARE now synchronizes the executor with the
parser-created context before parameter decoding.
- Parser and analyzer tests cover parameter retention, quoted question
marks, case-insensitive properties, constant-only property rejection,
repeated execution with different vectors and snapshots, mixed
constants/parameters, and invalid or missing parameters.
- A JDBC regression reuses one `ServerPreparedStatement` with A/B/A
parameter changes and compares results with literal SQL for both indexed
and flat searches. On live non-master FEs, it also prepares locally and
forces repeated executions to the master.
- A protocol test exercises five executions through the follower
decoder, RPC request construction, Thrift serialization, and a fresh
proxy decoder. It covers omitted type tables, unsigned values, NULL
parameters across two bitmap bytes, and later parameter type changes.
Local execution retains the original payload without copying it.
- PREPARE and EXECUTE share single-vector and multi-vector schema
validation. Tests cover schema-only PREPARE, multi-vector binding, and
rejection of invalid dimensions and excessive candidate budgets at
EXECUTE. The four CI FE UT failures caused by unbound-vector validation
were reproduced before the fix.
- Local verification: all 154 tests passed via `run-fe-ut.sh` across
`MysqlPreparedStatementForwardingTest`,
`MysqlConnectProcessorCursorFetchTest`, `FEOpExecutorMysqlProtocolTest`,
`LancePreparedSearchTest`, `LancePreparedStatementTest`,
`LanceVectorQueryTest`, `VectorSearchTableValuedFunctionTest`,
`FullTextSearchTableValuedFunctionTest`, `LanceScanNodeTest`,
`PrepareTest`, and `NereidsParserTest`. FE Checkstyle passed. The full
external JDBC regression was also executed locally using the current PR
FE build, a master FE, an observer FE, one BE, and the MinIO/Lance
fixtures: 1 suite passed, 0 failures, 0 skips. It executed 12 real
server-prepared queries covering direct and forwarded execution, indexed
and flat search, and A/B/A parameter changes. The original script
reproduced the CI String.positive() failure before fixing both JDBC URL
concatenations.

### Check List (For Author)

- Test
    - [x] Regression test
    - [x] Unit Test
- Behavior changed:
- [x] Yes. Supported `vector_search` property values can be prepared
parameters.
- Does this need documentation?
- [x] Yes. Usage and the supported parameter scope are documented above.

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
morningman added a commit that referenced this pull request Nov 5, 2026
…meter types (#70150)

### What problem does this PR solve?

Issue Number: None

Related PR: #68162, #69314

Problem Summary:

**In short.** This ports `MysqlPreparedStatementForwardingTest` from
branch-4.1 #68162 to master. Master already has the fix the test guards,
through #69314. The port adds what master's own tests do not cover: a
NULL bitmap of two bytes, and the forward request that `FEOpExecutor`
builds and Thrift carries. The PR changes tests only.

**Background.** A client that runs a server-side prepared statement
again may leave out the parameter types (`new_params_bind_flag` = 0) and
have the server reuse the ones it cached. A follower forwards every
COM_STMT_EXECUTE to the master. The master prepares the statement again
for each forwarded execution, so it has no cached types and cannot bind
the parameters. #68162 fixed this on branch-4.1, together with the Lance
prepared vector search it was written for. Master fixed it independently
in #69314: when the client reuses cached types,
`MysqlConnectProcessor.handleExecute` writes the forwarded packet again,
with the NULL bitmap, `new_params_bind_flag` = 1, every type code
including the unsigned flag, and the values. #69314 also has the prepare
stage plan with the context of the re-parsed statement, which is the
other generic part of #68162. Its tests in
`MysqlConnectProcessorLongDataTest` cover cached types, unsigned flags,
changing NULLs, changing types, all-NULL executions and the prepare
context.

**What those tests leave out.** They use at most four parameters, so the
NULL bitmap always fits in one byte. They also give the master the
buffer kept on the follower's context, rather than the request
`FEOpExecutor.buildStmtForwardParams` builds.

**What this PR adds.** The test runs a ten-parameter statement five
times on a follower connection:

- The parameter types are sent with the first and the fourth execution;
the other executions reuse the cached ones.
- One execution has a NULL parameter in each byte of the NULL bitmap.
- An unsigned INT parameter becomes a BIGINT in the fourth execution.

After each execution, the test builds the forward request with
`FEOpExecutor`, passes it through a Thrift round trip and executes it on
a new proxy context whose placeholders have no types. It then checks
three things: every parameter binds to the same value as on the
follower, the unsigned flag carries over, and the client's packet is
read to its end and left unchanged. The code is the 4.1 test adapted to
master's structure: master's `PrepareCommand`, query id and audit setup,
as in `MysqlConnectProcessorLongDataTest`, and expressions compared
directly. The 4.1 assertion that the follower keeps the client's buffer
was dropped, because master writes a separate buffer for cached types by
design.

**Results.**

| `handleExecute` on master | `MysqlConnectProcessorLongDataTest` (14) |
`MysqlPreparedStatementForwardingTest` (1) |
|---|---|---|
| as is | pass | pass |
| no rewrite of the forwarded packet for cached types (the bug #68162
fixed) | 3 fail | fails: the master binds untyped placeholders (`No
value present`) |
| the rewrite keeps only the first byte of the NULL bitmap | pass |
fails: the master reads the packet one byte off |

Co-authored-by: Gabriel <[email protected]>
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.

4 participants