feat: factor out advanced_extension
logic and add it to other ops
#65
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR factors out the handling of the
shared_extension
field from theplan
op and adds that logic to theproject
op. This mainly consisted of moving the import and export logic from the functions related to theplan
op to dedicated functions. The PR also introduces the newExtensibleOpInterface
that enforces an attribute calledadvanced_extension
on the op that implement it and allows to deal with all such ops transparently. Since that interface depends on an attribute, the include order of the generated code of interfaces and attributes also had to be adapted. Unfortunately, the field names in the Substrait spec also vary (singular or plural), so the PR also introduces some template magic to be able to deal with protobuf message types with both spellings. With this PR, message types with anadvanced_extension
field should be able to support it by (1) adding theExtensibleOpInterface
to their traits, (2) adding anadvanced_extension
parameter, and (3) adding that parameter to their assembly format (although that's technically optional; otherwise, the attribute is set through theattributes
dictionary).