Skip to content

[core] Enable multiple vector index columns on primary-key tables - #10755

Open
tzphh wants to merge 2 commits into
apache:masterfrom
tzphh:feature/pk-multiple-vector-indexes-cr2
Open

tzphh wants to merge 2 commits into
apache:masterfrom
tzphh:feature/pk-multiple-vector-indexes-cr2

Conversation

@tzphh

@tzphh tzphh commented Oct 22, 2026 •

Copy link
Copy Markdown
Contributor

Purpose

Related to #10658. Part 2 of the implementation series.
Depends on #10688.

Enable multiple columns in pk-vector.index.columns and validate each
column's type, distance metric, and build options independently. Queries
continue to select one column; VECTOR columns outside the index list remain
unindexed. Remove the unused singular column accessor and its direct tests.

Cover independent queries after compaction and reopen, updates and deletes,
different dimensions and metrics, and backfilling an added index on existing
data. Update the multi-column configuration and query documentation.

This branch includes the prerequisite commit from #10688. For the changes
specific to this PR, review commit 7136275025.
The branch will be rebased after the prerequisite is merged.

The updated prerequisite includes the writer test correction for the
maintainer list, so this PR can pass independently of the parallel-build PR.

Tests

  • 69 focused Core tests passed on the updated CR2 head, covering the
    writer reflection regression, maintainer lifecycle, vector writes,
    configuration, schema validation, and vector search.
  • Spotless, Checkstyle, Enforcer, and diff checks passed.
  • Local validation used JDK 17 and Maven 3.9.10. Flink/Spark engine suites
    were not rerun for this test-only review fix.

public final class BucketedPrimaryKeyIndexMaintainer {

@Nullable private final BucketedVectorIndexMaintainer vectorMaintainer;
private final List<BucketedVectorIndexMaintainer> vectorMaintainers;

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.

PrimaryKeyIndexWriteTest.testCreatesCoordinatorForMultipleScalarDefinitionsAndVector still reflects the old vectorMaintainer field name. Because this field was renamed to vectorMaintainers, both the JDK 8 and JDK 11 Core CI jobs fail with NoSuchFieldException at PrimaryKeyIndexWriteTest.java:142. Please update the test to read the list field, assert its expected size, and use the appropriate list element when checking deletionVectorFactory.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in the updated CR2 head 7136275, which includes the corrected prerequisite. The test reads vectorMaintainers as a list, checks its single entry, and retains the deletion-vector factory assertion. All 69 focused Core tests passed locally on this head (JDK 17).

I also verified the combined head e787eb7, which includes CR1 and CR2, on JDK 8 (1.8.0_512) and JDK 11 (11.0.27). All 139 focused Core tests passed on each runtime, including this regression test.

@JingsongLi

Copy link
Copy Markdown
Contributor

Requirement fit: SUPPORTED. Independent indexes for image/text embeddings have a concrete end-to-end use case; I found no additional production-code defect in the multi-column schema, restore, backfill, or query-selection paths.

I independently reproduced the reflection-test failure already reported above on a0fb04f (JDK 8): PrimaryKeyIndexWriteTest.testCreatesCoordinatorForMultipleScalarDefinitionsAndVector throws NoSuchFieldException: vectorMaintainer at line 142. This head is still blocked on that test update; the JDK 8 and JDK 11 Core CI jobs fail on the same case. Please carry the test correction into this prerequisite PR, rather than only the subsequent parallel-build PR.

Separate verification of the functional paths: 79 Core tests, 22 Flink vector-search procedure tests, and 13 Spark vector-search tests passed with normal Maven verification (including formatting, Checkstyle, Enforcer, and RAT). These include compaction/reopen, updates/deletes, independent dimensions/metrics, and adding an index to existing files. No duplicate inline finding added.

@tzphh
tzphh force-pushed the feature/pk-multiple-vector-indexes-cr2 branch from a0fb04f to 7136275 Compare October 27, 2026 07:08
@tzphh

tzphh commented Oct 27, 2026

Copy link
Copy Markdown
Contributor Author

CR2 has been rebased onto the corrected CR1 and updated to 7136275. This head now includes the reflection-test fix independently of CR3. All 69 focused Core tests passed locally, covering the writer regression, vector-index configuration/validation, and vector search (JDK 17).

For additional compatibility coverage, I tested the combined head e787eb7 on JDK 8 (1.8.0_512) and JDK 11 (11.0.27). All 139 focused Core tests passed on each runtime, including the reflection regression and multi-column vector-index tests.

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.

3 participants