Skip to content

feat(native): Add index source plan conversion with connector extensibility (#27596)#27596

Merged
zacw7 merged 1 commit intoprestodb:masterfrom
zacw7:export-D101022019
Apr 16, 2026
Merged

feat(native): Add index source plan conversion with connector extensibility (#27596)#27596
zacw7 merged 1 commit intoprestodb:masterfrom
zacw7:export-D101022019

Conversation

@zacw7
Copy link
Copy Markdown
Member

@zacw7 zacw7 commented Apr 15, 2026

Summary:

Add a virtual method toVeloxTableHandleForIndexSource to PrestoToVeloxConnector that allows connectors to customize how an IndexSourceNode is converted into a Velox ConnectorTableHandle. The default implementation delegates to toVeloxTableHandle, ignoring the IndexHandle. Connectors that support index lookup can override this to incorporate the IndexHandle into the resulting table handle, enabling index-aware table scan creation.

Without this extensibility point, the IndexHandle (carrying index column names resolved by the coordinator) would be silently dropped during worker-side plan conversion, and the worker would have no way to know which columns are indexed.

Also handles the case where IndexJoinOptimizer inserts a FilterNode between IndexJoinNode and IndexSourceNode for non-domain-expressible predicates. The FilterNode is unwrapped and its filter is merged into the join's remaining filter.

Changes:

  • PrestoToVeloxConnector.h: Add toVeloxTableHandleForIndexSource() virtual method with default delegation to toVeloxTableHandle().
  • PrestoToVeloxQueryPlan.cpp: Add toConnectorTableHandleForIndexSource() free function, update IndexSourceNode conversion to use it, add VELOX_CHECK_NOT_NULL guard, and add FilterNode unwrapping for IndexJoinNode.
  • PlanConverterTest.cpp: Add indexSource test covering IndexSourceNode to TableScanNode conversion.

Release Notes

== NO RELEASE NOTE ==

@zacw7 zacw7 requested review from a team as code owners April 15, 2026 20:36
@prestodb-ci prestodb-ci added the from:Meta PR from Meta label Apr 15, 2026
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai Bot commented Apr 15, 2026

Reviewer's Guide

Adds an extensibility point for connector-specific IndexSourceNode-to-ConnectorTableHandle conversion and wires IndexSourceNode plan conversion to use it, plus a regression test ensuring default behavior remains equivalent to a normal table scan producing a HiveTableHandle.

Sequence diagram for IndexSourceNode to ConnectorTableHandle conversion with index handle

sequenceDiagram
  participant PlanConverter as VeloxQueryPlanConverterBase
  participant IndexNode as protocol_IndexSourceNode
  participant Helper as toConnectorTableHandleForIndexSource
  participant ConnectorRegistry as PrestoToVeloxConnectorRegistry
  participant Connector as PrestoToVeloxConnector
  participant TableScan as core_TableScanNode

  PlanConverter->>PlanConverter: VELOX_CHECK_NOT_NULL(node)
  PlanConverter->>IndexNode: read outputVariables, assignments, tableHandle, indexHandle
  PlanConverter->>Helper: toConnectorTableHandleForIndexSource(tableHandle, indexHandle, exprConverter, typeParser)
  Helper->>ConnectorRegistry: getPrestoToVeloxConnector(tableHandle.connectorHandle._type)
  ConnectorRegistry-->>Helper: connector
  Helper->>Connector: toVeloxTableHandleForIndexSource(tableHandle, indexHandle, exprConverter, typeParser)
  alt default_implementation
    Connector-->>Helper: toVeloxTableHandle(tableHandle, exprConverter, typeParser)
  else index_aware_connector
    Connector-->>Helper: index_aware ConnectorTableHandle
  end
  Helper-->>PlanConverter: connectorTableHandle
  PlanConverter->>TableScan: create core_TableScanNode(id, rowType, connectorTableHandle, assignments)
  TableScan-->>PlanConverter: TableScanNode instance
Loading

Updated class diagram for PrestoToVeloxConnector and IndexSourceNode plan conversion

classDiagram
  class PrestoToVeloxConnector {
    <<interface>>
    +toVeloxTableHandle(tableHandle: protocol_TableHandle, exprConverter: VeloxExprConverter, typeParser: TypeParser) ConnectorTableHandle
    +toVeloxTableHandleForIndexSource(tableHandle: protocol_TableHandle, indexHandle: protocol_IndexHandle, exprConverter: VeloxExprConverter, typeParser: TypeParser) ConnectorTableHandle
    +toVeloxInsertTableHandle(tableHandle: protocol_TableHandle, exprConverter: VeloxExprConverter, typeParser: TypeParser) ConnectorInsertTableHandle
  }

  class VeloxQueryPlanConverterBase {
    -exprConverter_: VeloxExprConverter
    -typeParser_: TypeParser
    +toVeloxQueryPlan(node: protocol_IndexSourceNode, tableWriteInfo: TableWriteInfo, taskId: TaskId) core_PlanNode
  }

  class PlanConversionHelpers {
    +toConnectorTableHandle(tableHandle: protocol_TableHandle, exprConverter: VeloxExprConverter, typeParser: TypeParser) ConnectorTableHandle
    +toConnectorTableHandleForIndexSource(tableHandle: protocol_TableHandle, indexHandle: protocol_IndexHandle, exprConverter: VeloxExprConverter, typeParser: TypeParser) ConnectorTableHandle
  }

  class protocol_IndexSourceNode {
    +id: string
    +outputVariables: VariableList
    +assignments: AssignmentMap
    +tableHandle: protocol_TableHandle
    +indexHandle: protocol_IndexHandle
  }

  class protocol_TableHandle {
    +connectorHandle: ConnectorHandle
  }

  class protocol_IndexHandle {
    +indexColumns: ColumnList
  }

  class core_TableScanNode {
    +id: string
    +rowType: RowType
    +tableHandle: ConnectorTableHandle
    +assignments: ColumnHandleMap
  }

  class ConnectorTableHandle
  class ConnectorInsertTableHandle
  class VeloxExprConverter
  class TypeParser
  class TableWriteInfo
  class TaskId
  class core_PlanNode

  class ConcreteConnector {
    +toVeloxTableHandleForIndexSource(tableHandle: protocol_TableHandle, indexHandle: protocol_IndexHandle, exprConverter: VeloxExprConverter, typeParser: TypeParser) ConnectorTableHandle
  }

  VeloxQueryPlanConverterBase --> PlanConversionHelpers : calls
  PlanConversionHelpers --> PrestoToVeloxConnector : calls
  protocol_IndexSourceNode --> protocol_TableHandle : has
  protocol_IndexSourceNode --> protocol_IndexHandle : has
  core_TableScanNode --> ConnectorTableHandle : has
  PrestoToVeloxConnector <|.. ConcreteConnector
Loading

File-Level Changes

Change Details Files
Introduce connector hook for IndexSourceNode-specific table handle creation and default behavior.
  • Add virtual toVeloxTableHandleForIndexSource() to PrestoToVeloxConnector, taking a TableHandle and IndexHandle along with existing converter and parser utilities.
  • Provide default implementation that delegates to existing toVeloxTableHandle(), ignoring the IndexHandle so current connectors keep existing behavior unless overridden.
  • Document that connectors supporting index lookup should override this method to incorporate IndexHandle into the resulting ConnectorTableHandle.
presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.h
Wire IndexSourceNode conversion to use the new connector hook and ensure node non-nullity.
  • Add free function toConnectorTableHandleForIndexSource() that resolves the connector from the table handle’s connector type and calls its toVeloxTableHandleForIndexSource().
  • Update VeloxQueryPlanConverterBase::toVeloxQueryPlan(IndexSourceNode) to null-check the node with VELOX_CHECK_NOT_NULL.
  • Change IndexSourceNode conversion to call toConnectorTableHandleForIndexSource() with the node’s TableHandle and IndexHandle instead of the generic toConnectorTableHandle().
presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp
Add regression test and test data validating default IndexSourceNode conversion behavior.
  • Add PlanConverterTest.indexSource test that converts IndexSource.json and asserts the resulting plan contains a TableScanNode with expected id, output schema, and column assignments.
  • Verify that the resulting table handle is a HiveTableHandle pointing at tpch.nation, confirming default delegation behavior.
  • Introduce IndexSource.json test data describing an IndexSourceNode used by the new test.
presto-native-execution/presto_cpp/main/types/tests/PlanConverterTest.cpp
presto-native-execution/presto_cpp/main/types/tests/data/IndexSource.json

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai Bot left a comment

Choose a reason for hiding this comment

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

Hey - I've reviewed your changes and they look great!


Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@meta-codesync meta-codesync Bot changed the title feat(native): Add index source plan conversion with connector extensibility feat(native): Add index source plan conversion with connector extensibility (#27596) Apr 15, 2026
zacw7 added a commit to zacw7/presto that referenced this pull request Apr 15, 2026
…bility (prestodb#27596)

Summary:

Add a virtual method `toVeloxTableHandleForIndexSource` to `PrestoToVeloxConnector` that allows connectors to customize how an `IndexSourceNode` is converted into a Velox `ConnectorTableHandle`. The default implementation delegates to `toVeloxTableHandle`, ignoring the `IndexHandle`. Connectors that support index lookup can override this to incorporate the `IndexHandle` into the resulting table handle, enabling index-aware table scan creation.

Without this extensibility point, the `IndexHandle` (carrying index column names resolved by the coordinator) would be silently dropped during worker-side plan conversion, and the worker would have no way to know which columns are indexed.

Also updates `PrestoToVeloxQueryPlan::toVeloxQueryPlan(IndexSourceNode)` to call the new method, passing the `IndexHandle` through from the plan node.

**Changes:**

- `PrestoToVeloxConnector.h`: Add `toVeloxTableHandleForIndexSource()` virtual method with default delegation to `toVeloxTableHandle()`.
- `PrestoToVeloxQueryPlan.cpp`: Add `toConnectorTableHandleForIndexSource()` free function and update `IndexSourceNode` conversion to use it. Add `VELOX_CHECK_NOT_NULL` guard.

Differential Revision: D101022019
@zacw7 zacw7 force-pushed the export-D101022019 branch from 05855d5 to f5cc241 Compare April 15, 2026 20:52
@meta-codesync meta-codesync Bot changed the title feat(native): Add index source plan conversion with connector extensibility (#27596) feat(native): Add index source plan conversion with connector extensibility Apr 15, 2026
@zacw7 zacw7 force-pushed the export-D101022019 branch from f5cc241 to 87bdf7a Compare April 15, 2026 23:16
tanjialiang
tanjialiang previously approved these changes Apr 16, 2026
Copy link
Copy Markdown
Contributor

@tanjialiang tanjialiang left a comment

Choose a reason for hiding this comment

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

LGTM % nit

Comment thread presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.h Outdated
…bility (prestodb#27596)

Summary:

Add a virtual method `toVeloxTableHandleForIndexSource` to `PrestoToVeloxConnector` that allows connectors to customize how an `IndexSourceNode` is converted into a Velox `ConnectorTableHandle`. The default implementation delegates to `toVeloxTableHandle`, ignoring the `IndexHandle`. Connectors that support index lookup can override this to incorporate the `IndexHandle` into the resulting table handle, enabling index-aware table scan creation.

Without this extensibility point, the `IndexHandle` (carrying index column names resolved by the coordinator) would be silently dropped during worker-side plan conversion, and the worker would have no way to know which columns are indexed.

Also handles the case where `IndexJoinOptimizer` inserts a `FilterNode` between `IndexJoinNode` and `IndexSourceNode` for non-domain-expressible predicates. The `FilterNode` is unwrapped and its filter is merged into the join's remaining filter.

**Changes:**

- `PrestoToVeloxConnector.h`: Add `toVeloxTableHandleForIndexSource()` virtual method with default delegation to `toVeloxTableHandle()`.
- `PrestoToVeloxQueryPlan.cpp`: Add `toConnectorTableHandleForIndexSource()` free function, update `IndexSourceNode` conversion to use it, add `VELOX_CHECK_NOT_NULL` guard, and add `FilterNode` unwrapping for `IndexJoinNode`.
- `PlanConverterTest.cpp`: Add `indexSource` test covering `IndexSourceNode` to `TableScanNode` conversion.

Differential Revision: D101022019
@meta-codesync meta-codesync Bot changed the title feat(native): Add index source plan conversion with connector extensibility feat(native): Add index source plan conversion with connector extensibility (#27596) Apr 16, 2026
@zacw7 zacw7 force-pushed the export-D101022019 branch from 87bdf7a to 91023c0 Compare April 16, 2026 03:08
@zacw7 zacw7 merged commit 469a696 into prestodb:master Apr 16, 2026
124 of 128 checks passed
@zacw7 zacw7 deleted the export-D101022019 branch April 16, 2026 18:35
Copilot AI pushed a commit to Joe-Abraham/presto that referenced this pull request Apr 23, 2026
…bility (prestodb#27596) (prestodb#27596)

Summary:

Add a virtual method `toVeloxTableHandleForIndexSource` to
`PrestoToVeloxConnector` that allows connectors to customize how an
`IndexSourceNode` is converted into a Velox `ConnectorTableHandle`. The
default implementation delegates to `toVeloxTableHandle`, ignoring the
`IndexHandle`. Connectors that support index lookup can override this to
incorporate the `IndexHandle` into the resulting table handle, enabling
index-aware table scan creation.

Without this extensibility point, the `IndexHandle` (carrying index
column names resolved by the coordinator) would be silently dropped
during worker-side plan conversion, and the worker would have no way to
know which columns are indexed.

Also handles the case where `IndexJoinOptimizer` inserts a `FilterNode`
between `IndexJoinNode` and `IndexSourceNode` for non-domain-expressible
predicates. The `FilterNode` is unwrapped and its filter is merged into
the join's remaining filter.

**Changes:**

- `PrestoToVeloxConnector.h`: Add `toVeloxTableHandleForIndexSource()`
virtual method with default delegation to `toVeloxTableHandle()`.
- `PrestoToVeloxQueryPlan.cpp`: Add
`toConnectorTableHandleForIndexSource()` free function, update
`IndexSourceNode` conversion to use it, add `VELOX_CHECK_NOT_NULL`
guard, and add `FilterNode` unwrapping for `IndexJoinNode`.
- `PlanConverterTest.cpp`: Add `indexSource` test covering
`IndexSourceNode` to `TableScanNode` conversion.

## Release Notes
```
== NO RELEASE NOTE ==
```
Co-authored-by: Joe-Abraham <53977252+Joe-Abraham@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants