From 88f015fcd119843f675874e49f4b53b332e33935 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Thu, 4 Jun 2026 17:50:15 +0200 Subject: [PATCH 1/2] feat(extensions): support impl-level description in extensions 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 #810 Co-Authored-By: Claude Opus 4.8 (1M context) --- .../substrait/extension/SimpleExtension.java | 19 +++- .../extension/DescriptionExtensionTest.java | 93 +++++++++++++++++++ .../extensions/description_extensions.yaml | 45 +++++++++ 3 files changed, 154 insertions(+), 3 deletions(-) create mode 100644 core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java create mode 100644 core/src/test/resources/extensions/description_extensions.yaml diff --git a/core/src/main/java/io/substrait/extension/SimpleExtension.java b/core/src/main/java/io/substrait/extension/SimpleExtension.java index 1b3f05820..52e3884eb 100644 --- a/core/src/main/java/io/substrait/extension/SimpleExtension.java +++ b/core/src/main/java/io/substrait/extension/SimpleExtension.java @@ -274,6 +274,19 @@ public String description() { return ""; } + /** + * Resolves the effective description for this implementation. An implementation-level + * description (parsed into {@link #description()}) documents overload-specific behavior and + * takes precedence over the parent function's description when present. See substrait#1013. + */ + String resolveDescription(String functionDescription) { + String implDescription = description(); + return implDescription == null || implDescription.isEmpty() + ? functionDescription + : implDescription; + } + public abstract List args(); public abstract Map options(); @@ -403,7 +416,7 @@ public ScalarFunctionVariant resolve( return ImmutableSimpleExtension.ScalarFunctionVariant.builder() .urn(urn) .name(name) - .description(description) + .description(resolveDescription(description)) .nullability(nullability()) .args(args()) .options(options()) @@ -480,7 +493,7 @@ AggregateFunctionVariant resolve( return ImmutableSimpleExtension.AggregateFunctionVariant.builder() .urn(urn) .name(name) - .description(description) + .description(resolveDescription(description)) .nullability(nullability()) .args(args()) .options(options()) @@ -524,7 +537,7 @@ WindowFunctionVariant resolve( return ImmutableSimpleExtension.WindowFunctionVariant.builder() .urn(urn) .name(name) - .description(description) + .description(resolveDescription(description)) .nullability(nullability()) .args(args()) .options(options()) diff --git a/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java b/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java new file mode 100644 index 000000000..02ed1f964 --- /dev/null +++ b/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java @@ -0,0 +1,93 @@ +package io.substrait.extension; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import io.substrait.TestBase; +import java.io.IOException; +import java.io.UncheckedIOException; +import org.junit.jupiter.api.Test; + +/** + * Verifies that descriptions can be read from extension YAML files at both the function level and + * the function-implementation level. An implementation-level description documents + * overload-specific behavior and takes precedence over the parent function's description when both + * are present. + * + *

See substrait#1013. + */ +class DescriptionExtensionTest extends TestBase { + + static final String URN = "extension:test:description_extensions"; + static final SimpleExtension.ExtensionCollection DESCRIPTION_EXTENSION; + + static { + try { + String extensionStr = asString("extensions/description_extensions.yaml"); + DESCRIPTION_EXTENSION = SimpleExtension.load(extensionStr); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + DescriptionExtensionTest() { + super(DESCRIPTION_EXTENSION); + } + + @Test + void testFunctionLevelDescriptionInherited() { + // The single impl has no description of its own, so it inherits the function-level description. + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "documentedScalar:i64"); + assertEquals( + "Function-level scalar description", extensions.getScalarFunction(anchor).description()); + } + + @Test + void testImplLevelDescriptionTakesPrecedence() { + // The i64 overload defines its own description; it must win over the function-level one. + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "overloadDocumented:i64"); + assertEquals( + "Overload-specific description for i64", + extensions.getScalarFunction(anchor).description()); + } + + @Test + void testOverloadWithoutDescriptionInheritsFunctionLevel() { + // The fp32 overload has no description of its own, so it inherits the function-level one. + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "overloadDocumented:fp32"); + assertEquals( + "Shared function-level description", extensions.getScalarFunction(anchor).description()); + } + + @Test + void testImplLevelDescriptionWithoutFunctionLevel() { + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "implOnlyScalar:i64"); + assertEquals("Only the impl is documented", extensions.getScalarFunction(anchor).description()); + } + + @Test + void testAggregateImplLevelDescription() { + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "documentedAggregate:i64"); + assertEquals( + "Aggregate impl description", extensions.getAggregateFunction(anchor).description()); + } + + @Test + void testAggregateImplDescriptionCarriesOverToWindowFunction() { + // Aggregate functions are also registered as window functions; the description must carry over. + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "documentedAggregate:i64"); + assertEquals("Aggregate impl description", extensions.getWindowFunction(anchor).description()); + } + + @Test + void testWindowImplLevelDescriptionTakesPrecedence() { + SimpleExtension.FunctionAnchor anchor = + SimpleExtension.FunctionAnchor.of(URN, "documentedWindow:i64"); + assertEquals("Window impl description", extensions.getWindowFunction(anchor).description()); + } +} diff --git a/core/src/test/resources/extensions/description_extensions.yaml b/core/src/test/resources/extensions/description_extensions.yaml new file mode 100644 index 000000000..164fb7180 --- /dev/null +++ b/core/src/test/resources/extensions/description_extensions.yaml @@ -0,0 +1,45 @@ +%YAML 1.2 +--- +urn: extension:test:description_extensions +scalar_functions: + # Function-level description only; the single impl inherits it. + - name: "documentedScalar" + description: "Function-level scalar description" + impls: + - args: + - value: i64 + return: i64 + # The i64 overload documents itself and overrides the function-level + # description; the fp32 overload inherits the function-level description. + - name: "overloadDocumented" + description: "Shared function-level description" + impls: + - args: + - value: i64 + description: "Overload-specific description for i64" + return: i64 + - args: + - value: fp32 + return: fp32 + # Impl-level description with no function-level description. + - name: "implOnlyScalar" + impls: + - args: + - value: i64 + description: "Only the impl is documented" + return: i64 +aggregate_functions: + - name: "documentedAggregate" + impls: + - args: + - value: i64 + description: "Aggregate impl description" + return: i64 +window_functions: + - name: "documentedWindow" + description: "Window function-level description" + impls: + - args: + - value: i64 + description: "Window impl description" + return: i64 From 8b881e2604e4fe04eef740f2ffbe15fab42243e3 Mon Sep 17 00:00:00 2001 From: Niels Pardon Date: Fri, 12 Jun 2026 14:08:29 +0200 Subject: [PATCH 2/2] docs: remove PR links Signed-off-by: Niels Pardon --- core/src/main/java/io/substrait/extension/SimpleExtension.java | 3 +-- .../java/io/substrait/extension/DescriptionExtensionTest.java | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/core/src/main/java/io/substrait/extension/SimpleExtension.java b/core/src/main/java/io/substrait/extension/SimpleExtension.java index 52e3884eb..90892c3a6 100644 --- a/core/src/main/java/io/substrait/extension/SimpleExtension.java +++ b/core/src/main/java/io/substrait/extension/SimpleExtension.java @@ -277,8 +277,7 @@ public String description() { /** * Resolves the effective description for this implementation. An implementation-level * description (parsed into {@link #description()}) documents overload-specific behavior and - * takes precedence over the parent function's description when present. See substrait#1013. + * takes precedence over the parent function's description when present. */ String resolveDescription(String functionDescription) { String implDescription = description(); diff --git a/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java b/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java index 02ed1f964..03573a1ba 100644 --- a/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java +++ b/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java @@ -12,8 +12,6 @@ * the function-implementation level. An implementation-level description documents * overload-specific behavior and takes precedence over the parent function's description when both * are present. - * - *

See substrait#1013. */ class DescriptionExtensionTest extends TestBase {