substrait-io / substrait-java

Apache License 2.0
77 stars 72 forks source link

feat: opening ProtoRelConverter for extension #285

Closed bvolpato closed 3 months ago

bvolpato commented 3 months ago

I have a need to override some of the behavior in ProtoRelConverter.java's newExtensionTable, but today I can't because the class was designed to be all private.

I'd like to make that open if possible, to allow easily extending it instead of having to duplicate any code. I'm aware that there are some extension points / customization entries such as detailFromExtensionTable, but that signature isn't enough to have roundtrip-able leafs, because I can only derive the schema from the Rel itself, not from the data packed in the Any instance.

jacques-n commented 3 months ago

I generally suggest using composition instead of subclassing. Exposing these methods as protected creates some weird testing patterns as things progress. I'd be much more inclined to say let's convert to interface and final this class.