feat: Scheduler supports `substrait` logical plan and remove deprecated `sql` support #1360

mattcuento · 2026-01-02T23:10:13Z

Which issue does this PR close?

Closes #1358.
Part of #32.

Rationale for this change

As part of this epic, adding server-side support for Substrait plans (to the Scheduler ExecuteQuery API). Substrait support is gated by the substrait feature with conditional depdencies for datafusion-substrait and therefore substrait. This feature is on by default.

The sql field was marked as deprecated from the Ballista protocol two years back here. This is clean up of the field. The field ID and name have been marked as reserved to prevent corrupted requests if client/server versions drift across this change.

What changes are included in this PR?

Bumped base image to rust:1.92-trixie in order to pull an updated protobuf-compiler version that supports optional fields for proto3 by default.

Are there any user-facing changes?

milenkovicm · 2026-01-03T09:54:43Z

thanks @mattcuento, would you be interested in adding substrait as well as part of this PR?

mattcuento · 2026-01-03T16:13:08Z

thanks @mattcuento, would you be interested in adding substrait as well as part of this PR?

Sure thing!

mattcuento · 2026-01-04T03:14:42Z

Hey @milenkovicm, I added support for Substrait on ExecuteQuery. My assumption from the epic is that this is strictly intended to be a server-side change for use with different front-ends, and that we won't be changing the client to serialize into Substrait (sounds like that would be an unnecessary extra serialization step on top of LogicalPlan).

Assuming that's true, it was a little tricky to test this well aside from calling the scheduler RPC service directly (test_substrait_compatibility). I'm brand new to the project, so if you have any opinions/advice on where or how to better test this do let me know!

milenkovicm

I think change make sense, and test as well, thanks @mattcuento

there is one issue with docker build and looks like issue with disk space failing other (not sure how to fix)

ballista/scheduler/src/scheduler_server/grpc.rs

milenkovicm · 2026-01-04T14:25:54Z

Maybe as a follow up we should put a bit more documentation around this and example(s)

milenkovicm · 2026-01-04T17:33:24Z

Also, could we gate substrait with config option, which could be on by default?
Users not needing it could disable it at compile time.

mattcuento · 2026-01-05T02:45:40Z

there is one issue with docker build and looks like issue with disk space failing other (not sure how to fix)

Thanks, looks like substrait doesn't run a high enough version of protoc to to support optional fields by default.

error: failed to run custom build command for `substrait v0.62.2`
note: To improve backtraces for build dependencies, set the CARGO_PROFILE_RELEASE_BUILD_OVERRIDE_DEBUG=true environment variable to enable debug information generation.

Caused by:
  process didn't exit successfully: `/home/builder/workspace/target/release/build/substrait-339db7bdcba362ae/build-script-build` (exit status: 1)
  --- stdout
  cargo:rerun-if-env-changed=FORCE_REBUILD
  cargo:rerun-if-changed=substrait
  cargo:rerun-if-changed=substrait/text/dialect_schema.yaml
  cargo:rerun-if-changed=substrait/text/simple_extensions_schema.yaml
  cargo:rerun-if-changed=substrait/proto/substrait/plan.proto
  cargo:rerun-if-changed=substrait/proto/substrait/extensions/extensions.proto
  cargo:rerun-if-changed=substrait/proto/substrait/type.proto
  cargo:rerun-if-changed=substrait/proto/substrait/parameterized_types.proto
  cargo:rerun-if-changed=substrait/proto/substrait/algebra.proto
  cargo:rerun-if-changed=substrait/proto/substrait/extended_expression.proto
  cargo:rerun-if-changed=substrait/proto/substrait/capabilities.proto
  cargo:rerun-if-changed=substrait/proto/substrait/function.proto
  cargo:rerun-if-changed=substrait/proto/substrait/type_expressions.proto

  --- stderr
  Error: Custom { kind: Other, error: "protoc failed: substrait/type.proto: This file contains proto3 optional fields, but --experimental_allow_proto3_optional was not set.\n" }

From this issue I gathered that we could compile protoc ourselves for the build, and that's what I have now for the latest commit. However, looks like a few distributions don't ship with or download cmake for compilation. I'm curious, @milenkovicm, have any advice here? Should I just add cmake in the necessary Dockerfiles?

Maybe as a follow up we should put a bit more documentation around this and example(s)

Agreed, happy to file an issue to track it. I'm still kind of curious if more changes would be desired for user ergonomics. Do we have existing examples of connecting to a scheduler besides the client? I'd imagine it might be useful to add some convenience/wrapper methods to create scheduler gRPC clients, such as combining create_grpc_client_connection + SchedulerGrpcClient::new(connection) like in distributed_query.rs:

    info!("Connecting to Ballista scheduler at {scheduler_url}");
    // TODO reuse the scheduler to avoid connecting to the Ballista scheduler again and again
    let connection = create_grpc_client_connection(scheduler_url, &grpc_config)
        .await
        .map_err(|e| DataFusionError::Execution(format!("{e:?}")))?;

    let mut scheduler = SchedulerGrpcClient::new(connection)
        .max_encoding_message_size(max_message_size)
        .max_decoding_message_size(max_message_size);

I don't know much about Ibis, but will take a look to see how it would integrate with this.

Also, could we gate substrait with config option, which could be on by default?
Users not needing it could disable it at compile time.

Done! Updated the PR description, added support/conditional dependencies under substrait

ballista/scheduler/Cargo.toml

milenkovicm

thanks @mattcuento changes make sense, we just need to fix build issue and we can merge this

Cargo.toml

reserving field id 2 and field name sql

milenkovicm · 2026-01-05T16:57:15Z

not sure why it does not start github actions, maybe you should rebase

mattcuento · 2026-01-05T17:22:43Z

Will review the latest test linux balista/crates failures this evening

milenkovicm · 2026-01-05T17:26:14Z

perhaps, action getting out of disk space ? looks like linker freaking out, which should not be related to your change

mattcuento · 2026-01-06T02:22:56Z

Ok it does seem that the issues for test linux crates and test linux balista are to do with runner disk space being full.

You can monitor disk space with df -h in a workflow step.

I've tried a bunch of different tricks to free space (actions/runner-images#2840 for example) but no luck. We run tests in a container so we don't get access to the root file system to clean much up. We could alternatively run tests outside of a container which allows us more freedom to delete things.

This is what space looks like prior to building/running tests:

Filesystem      Size  Used Avail Use% Mounted on
overlay          72G   56G   16G  78% /
tmpfs            64M     0   64M   0% /dev
shm              64M     0   64M   0% /dev/shm
/dev/root        72G   56G   16G  78% /__t
tmpfs           3.2G  1.2M  3.2G   1% /run/docker.sock
tmpfs           7.9G     0  7.9G   0% /proc/acpi
tmpfs           7.9G     0  7.9G   0% /proc/scsi
tmpfs           7.9G     0  7.9G   0% /sys/firmware

I also don't know what kind of plan apache repos are on. Could we potentially just use a larger runner class?

mattcuento · 2026-01-06T05:48:47Z

Looks like you can reclaim quite a lot of space when running outside containers

this was just for test linux balista and test linux crates

milenkovicm · 2026-01-06T09:22:22Z

would cargo clean before action make any difference?

milenkovicm · 2026-01-06T09:24:44Z

maybe @Huy1Ng has idea how to fix issue with build, he did a great job reorganising github actions in #1289

milenkovicm · 2026-01-06T10:22:43Z

milenkovicm@b60d245 adding cargo clean may help

Huy1Ng · 2026-01-06T15:07:42Z

I couldn't replicate the error on my fork, but add this to the CI section in Cargo.toml

[profile.ci]
inherits = "dev"
incremental = false
debug = false
debug-assertions = false
strip = "debuginfo"

reduce the ./target directory size by ~10%. I'm not sure how cargo clean would help as we should start with empty ./target for each job (or do we?!)

milenkovicm · 2026-01-06T15:19:02Z

I have tested with cargo clean in https://github.com/milenkovicm/datafusion-ballista/tree/check__remove-query-type-sql-1358 it worked there, i'm not sure if it is accident.

mattcuento · 2026-01-06T15:47:37Z

Interesting, @milenkovicm, I tried cargo clean prior to tests yesterday while checking disk usage and we didn't seem to reclaim anything.

However, I did just try @Huy1Ng suggestion for the CI profile and that seemed to work. I've cleaned up some commit history now, hopefully should be ready to merge.

Thank you both for the suggestions here 🙂

milenkovicm

thanks @mattcuento
one minor comment

milenkovicm · 2026-01-06T16:07:28Z

ballista/scheduler/src/scheduler_server/grpc.rs

-                        .sql(&sql)
-                        .await
-                        .and_then(|df| df.into_optimized_plan())
+                Query::SubstraitPlan(bytes) => {


warning: unused variable: `bytes` --> ballista/scheduler/src/scheduler_server/grpc.rs:401:38 | 401 | Query::SubstraitPlan(bytes) => { | ^^^^^ help: if this is intentional, prefix it with an underscore: `_bytes` | = note: `#[warn(unused_variables)]` (part of `#[warn(unused)]`) on by default

if substrait disabled

Thanks, fixed!

Linting + using non-reserved field id Adding test for substrait compatibility with local scheduler instance add inline expressio to substrait test protect substrait support under feature Adding protoc feature to datafusion-substrait package Removing protoc feature Bump rust base version Rebuild Cargo.lock spacing spacing Add cargo clean before test remove cargo clean Fix linting for feature off

milenkovicm · 2026-01-06T20:51:00Z

great effort, thanks a lot @mattcuento
I hope you will follow up with docs and examples

mattcuento force-pushed the remove-query-type-sql-1358 branch 3 times, most recently from 7119f99 to 887096b Compare January 4, 2026 02:58

mattcuento changed the title ~~chore: Remove deprecated Sql field from ExecuteQueryParams.Query~~ feat: support substrait_plan and remove deprecated sql field from ExecuteQueryParams.Query Jan 4, 2026

mattcuento mentioned this pull request Jan 4, 2026

[EPIC] Add support for Substrait #32

Closed

3 tasks

milenkovicm reviewed Jan 4, 2026

View reviewed changes

ballista/scheduler/src/scheduler_server/grpc.rs Outdated Show resolved Hide resolved

ballista/scheduler/src/scheduler_server/grpc.rs Show resolved Hide resolved

mattcuento force-pushed the remove-query-type-sql-1358 branch 2 times, most recently from 1816b45 to f75d42e Compare January 5, 2026 01:55

milenkovicm reviewed Jan 5, 2026

View reviewed changes

ballista/scheduler/Cargo.toml Show resolved Hide resolved

milenkovicm reviewed Jan 5, 2026

View reviewed changes

Cargo.toml Outdated Show resolved Hide resolved

mattcuento force-pushed the remove-query-type-sql-1358 branch 2 times, most recently from c68a693 to ed838e9 Compare January 5, 2026 16:46

chore: Remove deprecated Sql field from ExecuteQueryParams.Query

d31e42f

reserving field id 2 and field name sql

mattcuento force-pushed the remove-query-type-sql-1358 branch 2 times, most recently from 1697215 to b04b37b Compare January 5, 2026 17:01

mattcuento marked this pull request as ready for review January 5, 2026 17:07

mattcuento mentioned this pull request Jan 5, 2026

feat: Bump docker rust to rust:1.92-trixie #1365

Merged

mattcuento force-pushed the remove-query-type-sql-1358 branch from b04b37b to 12c2c61 Compare January 6, 2026 02:19

mattcuento force-pushed the remove-query-type-sql-1358 branch from 12c2c61 to c6b02f8 Compare January 6, 2026 03:33

mattcuento force-pushed the remove-query-type-sql-1358 branch 6 times, most recently from 695ff88 to 3879489 Compare January 6, 2026 05:44

mattcuento force-pushed the remove-query-type-sql-1358 branch 2 times, most recently from c6b02f8 to 8706cd3 Compare January 6, 2026 15:04

mattcuento force-pushed the remove-query-type-sql-1358 branch from 72a8639 to 72b3db6 Compare January 6, 2026 15:44

milenkovicm approved these changes Jan 6, 2026

View reviewed changes

mattcuento added 2 commits January 6, 2026 10:45

chore: Edit ci profile to reduce ./target dir size

c66b160

mattcuento force-pushed the remove-query-type-sql-1358 branch from 72b3db6 to c66b160 Compare January 6, 2026 18:45

milenkovicm changed the title ~~feat: support substrait_plan and remove deprecated sql field from ExecuteQueryParams.Query~~ feat: scheduler supports substrait logical and remove deprecated sql support Jan 6, 2026

milenkovicm changed the title ~~feat: scheduler supports substrait logical and remove deprecated sql support~~ feat: Scheduler supports substrait logical and remove deprecated sql support Jan 6, 2026

milenkovicm changed the title ~~feat: Scheduler supports substrait logical and remove deprecated sql support~~ feat: Scheduler supports substrait logical plan and remove deprecated sql support Jan 6, 2026

milenkovicm merged commit 60182c8 into apache:main Jan 6, 2026
31 checks passed

mattcuento mentioned this pull request Jan 6, 2026

Add documentation/examples on how to use substrait plans with Ballista #1368

Open

feat: Scheduler supports substrait logical plan and remove deprecated sql support #1360

feat: Scheduler supports substrait logical plan and remove deprecated sql support #1360

Uh oh!

Conversation

mattcuento commented Jan 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are there any user-facing changes?

Uh oh!

milenkovicm commented Jan 3, 2026

Uh oh!

mattcuento commented Jan 3, 2026

Uh oh!

mattcuento commented Jan 4, 2026

Uh oh!

milenkovicm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

milenkovicm commented Jan 4, 2026

Uh oh!

milenkovicm commented Jan 4, 2026

Uh oh!

mattcuento commented Jan 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

milenkovicm left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

milenkovicm commented Jan 5, 2026

Uh oh!

mattcuento commented Jan 5, 2026

Uh oh!

milenkovicm commented Jan 5, 2026

Uh oh!

mattcuento commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mattcuento commented Jan 6, 2026

Uh oh!

milenkovicm commented Jan 6, 2026

Uh oh!

milenkovicm commented Jan 6, 2026

Uh oh!

milenkovicm commented Jan 6, 2026

Uh oh!

Huy1Ng commented Jan 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

milenkovicm commented Jan 6, 2026

Uh oh!

mattcuento commented Jan 6, 2026

Uh oh!

milenkovicm left a comment

Choose a reason for hiding this comment

Uh oh!

milenkovicm Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

mattcuento Jan 6, 2026

Choose a reason for hiding this comment

Uh oh!

milenkovicm commented Jan 6, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

feat: Scheduler supports `substrait` logical plan and remove deprecated `sql` support #1360

feat: Scheduler supports `substrait` logical plan and remove deprecated `sql` support #1360

mattcuento commented Jan 2, 2026 •

edited

Loading

mattcuento commented Jan 5, 2026 •

edited

Loading

mattcuento commented Jan 6, 2026 •

edited

Loading

Huy1Ng commented Jan 6, 2026 •

edited

Loading