diff --git a/core/src/main/java/io/substrait/extension/SimpleExtension.java b/core/src/main/java/io/substrait/extension/SimpleExtension.java index 1b3f05820..90892c3a6 100644 --- a/core/src/main/java/io/substrait/extension/SimpleExtension.java +++ b/core/src/main/java/io/substrait/extension/SimpleExtension.java @@ -274,6 +274,18 @@ 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. + */ + String resolveDescription(String functionDescription) { + String implDescription = description(); + return implDescription == null || implDescription.isEmpty() + ? functionDescription + : implDescription; + } + public abstract List args(); public abstract Map options(); @@ -403,7 +415,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 +492,7 @@ AggregateFunctionVariant resolve( return ImmutableSimpleExtension.AggregateFunctionVariant.builder() .urn(urn) .name(name) - .description(description) + .description(resolveDescription(description)) .nullability(nullability()) .args(args()) .options(options()) @@ -524,7 +536,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..03573a1ba --- /dev/null +++ b/core/src/test/java/io/substrait/extension/DescriptionExtensionTest.java @@ -0,0 +1,91 @@ +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. + */ +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