Skip to content

feat(core): add typeVariationReference to UserDefined type#794

Open
kadinrabo wants to merge 1 commit intosubstrait-io:mainfrom
kadinrabo:kr/type-variation-reference
Open

feat(core): add typeVariationReference to UserDefined type#794
kadinrabo wants to merge 1 commit intosubstrait-io:mainfrom
kadinrabo:kr/type-variation-reference

Conversation

@kadinrabo
Copy link
Copy Markdown
Contributor

What does this PR do?

Adds typeVariationReference support to the Type.UserDefined POJO, closing the gap where the Substrait proto defines type_variation_reference (field #2) on UserDefined but substrait-java ignores it entirely.

Downstream users who need type variations on UDTs (e.g., specialized execution engine variants) previously had to drop to raw proto manipulation. This change threads the field through the full POJO ↔ proto roundtrip.

Closes #567

Changes

  • Type.java — Add typeVariationReference() to UserDefined (abstract + builder default, see note below)
  • ProtoTypeConverter — Read getTypeVariationReference() from proto when building POJO
  • BaseProtoTypes / TypeProtoConverter / BaseProtoConverter — Thread typeVariationReference through POJO→proto conversion
  • TypeCreator / SubstraitBuilder — Add overloads accepting typeVariationReference
  • TypeExtensionTest — Roundtrip test verifying non-zero variation reference survives proto conversion, plus default-value assertion

Note on Immutables workaround

typeVariationReference uses abstract + builder factory default instead of @Value.Default due to an Immutables codegen bug: when a @Value.Enclosing type has multiple @Value.Default fields in a nested class, the generated InitShim references UserDefined.super from a non-subclass context. The workaround achieves identical ergonomics — callers don't need to set the field explicitly.

Test plan

  • New roundtripCustomTypeWithVariationReference test: creates UDT with typeVariationReference=42, roundtrips through proto, verifies both proto-level field value and full POJO equality
  • Existing roundtripCustomType test: added assertion that default typeVariationReference is 0
  • Full test suite passes (core, isthmus, spark)

The Substrait proto defines type_variation_reference on UserDefined but
substrait-java ignores it. Add the field to the POJO, thread it through
both proto converters, and add builder/DSL overloads.

Closes substrait-io#567
@kadinrabo
Copy link
Copy Markdown
Contributor Author

This addresses the UserDefined gap described in #567. While working on this I noticed that type_variation_reference is actually ignored for all built-in types as well (Bool, I8, I16, I32, I64, FP32, FP64, String, Binary, Date, Time, Timestamp, etc.) — the proto field exists on every type message but substrait-java drops it during conversion in both directions.

Happy to open a follow-up PR for comprehensive type_variation_reference support across all types if that would be useful.

@kadinrabo kadinrabo marked this pull request as ready for review April 3, 2026 17:37
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.

type_variation_reference not implemented for User Defined Types in substrait-java

1 participant