Skip to content

Added substrait converter for the fragments - #21348

Merged
mch2 merged 4 commits into
opensearch-project:mainfrom
vinaykpud:feature/datanode-execution
Apr 24, 2026
Merged

Added substrait converter for the fragments#21348
mch2 merged 4 commits into
opensearch-project:mainfrom
vinaykpud:feature/datanode-execution

Conversation

@vinaykpud

@vinaykpud vinaykpud commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Description

Add Substrait-based fragment converter to the DataFusion backend plugin, enabling end-to-end query execution from DSL query → Calcite RelNode → Substrait plan → native DataFusion runtime → Arrow result batches → materialized rows.

  • Fix classloader dependency issues: flatbuffers-java, arrow-format, jackson-databind, jackson-annotations moved to bundled in analytics-engine (parent plugin). Arrow's ArrowType subclasses are loaded by the parent classloader and transitively reference these jars — child plugins can't provide them. Removed duplicates from analytics-backend-datafusion to avoid jar hell.

  • Fix Substrait TCCL issue: Substrait uses Jackson polymorphic deserialization which resolves class names via the thread context classloader. On search threads the TCCL is the system CL and can't see plugin classes. Moved extension catalog loading to plugin bootstrap with a one-time TCCL swap so it's cached before any query runs.

  • Fix Arrow memory leak: batchesToRows() iterated VectorSchemaRoot batches without closing them. Added batch.close() after materializing rows so the per-query allocator closes cleanly.

Related Issues

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • Functionality includes testing.
  • API changes companion pull request created, if applicable.
  • Public documentation issue/PR created, if applicable.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@github-actions

This comment was marked as resolved.

@vinaykpud vinaykpud added skip-changelog skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis. labels Apr 24, 2026
@vinaykpud
vinaykpud force-pushed the feature/datanode-execution branch from 7b3fd7a to c5398f2 Compare April 24, 2026 06:47
@vinaykpud
vinaykpud marked this pull request as ready for review April 24, 2026 07:09
@vinaykpud
vinaykpud requested a review from a team as a code owner April 24, 2026 07:09
@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for c5398f2: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@mch2 mch2 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

overall looks like a good start here for substrait conversion, thanks @vinaykpud .

@sandeshkr419

Copy link
Copy Markdown
Member
Execution failed for task ':sandbox:plugins:analytics-backend-datafusion:thirdPartyAudit'.
> Audit of third party dependencies failed

@vinaykpud lets fix this as well. Thanks!

@github-actions

Copy link
Copy Markdown
Contributor

❌ Gradle check result for 302985b: FAILURE

Please examine the workflow log, locate, and copy-paste the failure(s) below, then iterate to green. Is the failure a flaky test unrelated to your change?

@vinaykpud

Copy link
Copy Markdown
Contributor Author

@sandeshkr419
Sure.
The missing class SqlDdlParserImpl is Calcite's DDL parser,it handles SQL statements like CREATE TABLE, DROP TABLE, ALTER TABLE. It lives in a separate calcite-server module and we don't need DDL parsing. So fixing by adding it to ignored list

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
@vinaykpud
vinaykpud force-pushed the feature/datanode-execution branch from e4c8905 to c39e324 Compare April 24, 2026 19:01
@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for c39e324: SUCCESS

@codecov

codecov Bot commented Apr 24, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.33%. Comparing base (436e4a6) to head (9a81b59).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@             Coverage Diff              @@
##               main   #21348      +/-   ##
============================================
- Coverage     73.34%   73.33%   -0.02%     
+ Complexity    74223    74205      -18     
============================================
  Files          5958     5958              
  Lines        337309   337349      +40     
  Branches      48664    48687      +23     
============================================
- Hits         247408   247380      -28     
- Misses        70188    70246      +58     
- Partials      19713    19723      +10     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
@github-actions

Copy link
Copy Markdown
Contributor

✅ Gradle check result for 9a81b59: SUCCESS

@mch2
mch2 merged commit 21d3a8a into opensearch-project:main Apr 24, 2026
26 of 32 checks passed
krishna-ggk pushed a commit to krishna-ggk/OpenSearch that referenced this pull request Apr 28, 2026
* Added substrait converter for the fragments

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* spotless fix

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* fixed pr comments

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* fix failing test

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

---------

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
Chocochip101 pushed a commit to Chocochip101/OpenSearch that referenced this pull request Apr 28, 2026
* Added substrait converter for the fragments

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* spotless fix

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* fixed pr comments

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* fix failing test

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

---------

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
imRishN pushed a commit to imRishN/OpenSearch that referenced this pull request May 8, 2026
* Added substrait converter for the fragments

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* spotless fix

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* fixed pr comments

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

* fix failing test

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>

---------

Signed-off-by: Vinay Krishna Pudyodu <vinkrish.neo@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip-changelog skip-diff-analyzer Maintainer to skip code-diff-analyzer check, after reviewing issues in AI analysis. skip-diff-reviewer Maintainer to skip code-diff-reviewer check, after reviewing issues in AI analysis.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants