Skip to content

feat: substrait builder extra api#773

Merged
nielspardon merged 10 commits into
substrait-io:mainfrom
mbwhite:additional-substrait-builder
Jun 8, 2026
Merged

feat: substrait builder extra api#773
nielspardon merged 10 commits into
substrait-io:mainfrom
mbwhite:additional-substrait-builder

Conversation

@mbwhite

@mbwhite mbwhite commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

Follow on to pr #767 to fill some missing gaps in the substrait builder. We've recently used the Substrait Builder and found that there some variants and missing gaps in the APIs. We'd subclassed it to add these, so I'm now wanting to push these to the main codebase.

There wasn't a unit test already so I've added a basic one here - which I appreciate will is quite large.

@github-actions

Copy link
Copy Markdown

ACTION NEEDED

Substrait follows the Conventional Commits
specification
for
release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

@mbwhite mbwhite force-pushed the additional-substrait-builder branch from bd62615 to d0cc543 Compare March 23, 2026 11:58
@mbwhite mbwhite changed the title Additional substrait builder feat: substrait builder extra api Mar 23, 2026
@mbwhite mbwhite force-pushed the additional-substrait-builder branch 3 times, most recently from 8618365 to 434df46 Compare March 24, 2026 10:29
@mbwhite mbwhite marked this pull request as ready for review March 24, 2026 10:34
@mbwhite mbwhite force-pushed the additional-substrait-builder branch from 434df46 to 83fa865 Compare March 24, 2026 11:50
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
@mbwhite mbwhite force-pushed the additional-substrait-builder branch 3 times, most recently from 5bdace0 to ef5e526 Compare March 24, 2026 13:10

@benbellick benbellick 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.

Just a few comments. Thanks!

Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java
@mbwhite

mbwhite commented Mar 25, 2026

Copy link
Copy Markdown
Contributor Author

@benbellick updated as suggested :-)

@mbwhite mbwhite requested a review from benbellick March 25, 2026 11:52
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
@mbwhite mbwhite force-pushed the additional-substrait-builder branch from efabd93 to fe68836 Compare March 25, 2026 13:15
@mbwhite mbwhite requested a review from benbellick March 26, 2026 09:28
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
@mbwhite mbwhite force-pushed the additional-substrait-builder branch from fe68836 to 1ca14b2 Compare March 27, 2026 11:51
@mbwhite mbwhite requested a review from benbellick April 2, 2026 10:27
@nielspardon

Copy link
Copy Markdown
Member

@benbellick since you started this review, do you have time for another look?

@benbellick benbellick 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.

Left a few more comments. Thanks!

Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java
Comment thread core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java Outdated
Comment thread core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
@mbwhite mbwhite force-pushed the additional-substrait-builder branch from acd20a0 to cbcf138 Compare June 5, 2026 10:35
@mbwhite

mbwhite commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

Sorry for the delay here.. other work commitments

@mbwhite mbwhite requested review from benbellick and nielspardon June 5, 2026 10:58
@mbwhite

mbwhite commented Jun 5, 2026

Copy link
Copy Markdown
Contributor Author

@benbellick my phone has told me there's a comment but I can't see in the PR... to answer your question - over enthaustic search and replace.

@benbellick benbellick 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.

Left a few comments but they are mostly about docs and the tests. Otherwise, I think its good. Thanks for your patience with my reviews!

Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java
Comment thread core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java
Comment thread core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java
Comment thread core/src/test/java/io/substrait/dsl/SubstraitBuilderTest.java
@benbellick

Copy link
Copy Markdown
Member

@benbellick my phone has told me there's a comment but I can't see in the PR... to answer your question - over enthaustic search and replace.

@mbwhite Sorry, I was trying to do a review but accidentally left that first comment by mistake.

@mbwhite mbwhite force-pushed the additional-substrait-builder branch from cbcf138 to d413105 Compare June 8, 2026 09:08
@mbwhite

mbwhite commented Jun 8, 2026

Copy link
Copy Markdown
Contributor Author

@benbellick @nielspardon update with the latest comments Ben. :-)

I'll take the example suggestion and do a large example PR

@mbwhite mbwhite force-pushed the additional-substrait-builder branch from 5249003 to 77dada1 Compare June 8, 2026 12:21
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
@mbwhite mbwhite force-pushed the additional-substrait-builder branch from 937a20b to 172d48a Compare June 8, 2026 12:49
Comment thread core/src/main/java/io/substrait/dsl/SubstraitBuilder.java Outdated
mbwhite and others added 10 commits June 8, 2026 15:31
Signed-off-by: MBWhite <whitemat@uk.ibm.com>
Signed-off-by: MBWhite <whitemat@uk.ibm.com>
Signed-off-by: MBWhite <whitemat@uk.ibm.com>
Signed-off-by: MBWhite <whitemat@uk.ibm.com>
Signed-off-by: matthew brian white <whitemat@uk.ibm.com>
Signed-off-by: matthew brian white <whitemat@uk.ibm.com>
Signed-off-by: matthew brian white <whitemat@uk.ibm.com>
Co-authored-by: Niels Pardon <mail@niels-pardon.de>
Signed-off-by: matthew brian white <whitemat@uk.ibm.com>
Signed-off-by: matthew brian white <whitemat@uk.ibm.com>
@mbwhite mbwhite force-pushed the additional-substrait-builder branch from 2e1ceb9 to 75a5722 Compare June 8, 2026 14:33
@mbwhite mbwhite requested a review from nielspardon June 8, 2026 14:34

@nielspardon nielspardon 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.

LGTM

@nielspardon nielspardon merged commit 594a168 into substrait-io:main Jun 8, 2026
11 checks passed
@mbwhite mbwhite deleted the additional-substrait-builder branch June 8, 2026 15:16
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