Skip to content

Conversation

@vbarua
Copy link
Member

@vbarua vbarua commented Dec 19, 2025

ConverterProvider is a new class consumed by the various classes used to convert
between SQL <-> Calcite <-> Substrait

Its usage allows us to centralize the configuration of conversions behaviours,
which both helps to:

  1. Provide clear standard defaults behaviours.
  2. Provide a clear location to extend behaviours.

Copy link
Member Author

@vbarua vbarua left a comment

Choose a reason for hiding this comment

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

I'm experimenting with overhauling the converter configuration to make changes like #647 easier to make, and to handle some of the concerns I had with #457 (review).

I still need to figure out how to structure the code to make it easy to utilize the DynamicConverterProvider, but I wanted to share the kernel of the idea.

import org.apache.calcite.rel.type.RelDataTypeFactory;
import org.apache.calcite.rex.RexBuilder;

public class ConverterProvider {
Copy link
Member Author

@vbarua vbarua Dec 19, 2025

Choose a reason for hiding this comment

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

We wire in the same 4 converters (3 function converter + 1 type converter) in a number of places (SubstraitRelVisitor, SubstraitRelNodeConverter, ...). Instead of doing that, we can centralized the configuration of converters into one provider*. This would also allow us to provide extended providers, like the DynamicConverterProvider, that could implement the dynamic fallback behaviour in a single location rather that sprinkling it throughout the various parts of the codebase, which makes it very difficult to reason about.

* I'm not set on the name

@github-actions
Copy link

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.

@vbarua vbarua force-pushed the vbarua/conversion-configuration-refactor branch from 94eab31 to 685de52 Compare December 22, 2025 19:00
@vbarua vbarua changed the title [WIP] experimenting with centralizing converter configurations feat: introduce ConverterProvider to control conversion behaviors Dec 23, 2025
@vbarua vbarua changed the title feat: introduce ConverterProvider to control conversion behaviors feat: introduce ConverterProvider to control conversion behaviour Dec 23, 2025
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.

2 participants