Skip to content

feat(extensions): support impl-level description in extensions#848

Merged
nielspardon merged 2 commits into
substrait-io:mainfrom
nielspardon:feat/extension-impl-description
Jun 12, 2026
Merged

feat(extensions): support impl-level description in extensions#848
nielspardon merged 2 commits into
substrait-io:mainfrom
nielspardon:feat/extension-impl-description

Conversation

@nielspardon

Copy link
Copy Markdown
Member

Summary

Substrait v0.86.0 added an optional description field to individual function implementations (substrait#1013). An impl-level description documents overload-specific behavior and can appear on each scalar/aggregate/window function implementation.

The variant already inherits description() from Function, so the impl-level value was being parsed but then discarded: resolve() unconditionally overwrote it with the parent function's description. This PR makes the impl-level description take precedence over the parent function's when present, mirroring the impl-level precedence used for deprecation info.

Closes #810

Changes

  • Add Function.resolveDescription(...) helper that prefers a non-empty impl-level description over the parent function's, and wire it into the scalar/aggregate/window resolve() methods.
  • Add DescriptionExtensionTest and a description_extensions.yaml fixture covering function-level-only (inherited), impl-level overriding function-level, sibling overloads (one overriding, one inheriting), impl-level with no function-level, and aggregate-as-window carry-over.

Testing

  • ./gradlew :core:test passes (including the new test).
  • ./gradlew :core:spotlessCheck passes.

🤖 Generated with Claude Code

Substrait v0.86.0 added an optional `description` field to individual
function implementations (substrait-io/substrait#1013). An impl-level
description documents overload-specific behavior and can appear on each
scalar/aggregate/window function implementation.

The variant already inherits `description()` from Function, so the
impl-level value was being parsed but then discarded: resolve()
unconditionally overwrote it with the parent function's description.
Make the impl-level description take precedence over the parent
function's when present, mirroring the impl-level precedence used for
deprecation info.

Closes substrait-io#810

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@nielspardon

Copy link
Copy Markdown
Member Author

Claude thought it makes sense to overwrite the function level description with the impl specific description. I guess this is reasonable.

@vbarua vbarua 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 some minor comments but other looks good to me.

This has the same lossy behaviour as #846 (overwriting the description in some cases), but is consistent with what we do today and we can improve it as a followup.

Comment thread core/src/main/java/io/substrait/extension/SimpleExtension.java
Comment thread core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java Outdated
Signed-off-by: Niels Pardon <par@zurich.ibm.com>
@nielspardon nielspardon merged commit eb453f2 into substrait-io:main Jun 12, 2026
11 checks passed
@nielspardon nielspardon deleted the feat/extension-impl-description branch June 12, 2026 12:41
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.

support optional description field to function implementations

2 participants