fix: remove ignored nullability markers from MIRROR declarations

[nielspardon]

Under MIRROR nullability the output nullability is derived from the arguments, so the spec requires that neither argument nor return types carry a ? marker, since it is ignored.

text/simple_extensions_schema.yaml:

MIRROR: Output is nullable if any argument is nullable; argument and return types must not include nullability markers as they will be ignored for output nullability derivation.

Seven impls in the standard catalog violated this. I enumerated them by loading every extensions/functions_*.yaml through a parser rather than grepping, so this is the complete set:

file	function	markers removed
functions_boolean.yaml	or	1 argument, return
functions_boolean.yaml	and	1 argument, return
functions_boolean.yaml	and_not	2 arguments, return
functions_boolean.yaml	xor	2 arguments, return
functions_boolean.yaml	not	1 argument, return
functions_rounding_decimal.yaml	ceil	return (decimal?<precision, 0>)
functions_rounding_decimal.yaml	floor	return (decimal?<precision, 0>)

bool_and, bool_or and round are left as-is: they declare DECLARED_OUTPUT, where the return marker is authoritative.

This is not a breaking change

Derived behavior is unchanged. With the markers ignored per spec, or(boolean, boolean) -> boolean and or(boolean?, boolean?) -> boolean?, which is what Kleene logic wants and what the existing test cases in tests/cases/boolean/ already assert. No plan that was legal before becomes illegal.

It does matter for implementations that validate a plan's declared output type against the declaration, though: a producer that reads return: boolean? literally will declare boolean? for or(boolean, boolean) and be rejected by a conforming validator. That inconsistency is what this fixes.

Regression guard

Also adds validate_nullability_markers plus a test over the shipped catalog, so the markers cannot be reintroduced. The check:

• skips DISCRETE, where markers are meaningful on both sides;
• only inspects outermost nullability, so nested markers like func<any1 -> boolean?> and list<i32?> still pass;
• reads the final line of a derivation expression, so a ? ternary in an intermediate line (scale = init_prec > 38 ? scale_after_borrow : init_scale) is not mistaken for a marker.

Verified it fails with a clear message when either marker is reintroduced.

Fixes #1041
Fixes #853

🤖 Generated with AI

---

This change is 

[nielspardon]

Breaking-change policy: no code impact in any active library

Per the breaking change policy, a breaking change needs a migration strategy implemented in all active libraries before it lands. I audited all four. None of them reads the declared marker under MIRROR, so none needs a migration.

library	how MIRROR output nullability is computed	reads declared ??
substrait-go	extensions/variants.go:155 — outType.WithNullability(Required/Nullable) from allNonNull, overriding the declared type	No. Arguments use MatchWithoutNullability for MIRROR/DECLARED_OUTPUT (variants.go:230)
substrait-java	isthmus SimpleExtensionToSqlOperator.determineNullability → opBinding.collectOperandTypes().anyMatch(RelDataType::isNullable)	No. Core SimpleExtension.validateOutputType is a disabled no-op (commented out as "too restrictive in the case of nullability conversion"), and resolveType has no callers in core
substrait-python	extension_registry/function_entry.py — after evaluate(...), overwrites output nullability to NULLABLE/REQUIRED based on the call signature	No. check_nullability is set true only for DISCRETE
substrait-rs	No derivation step; parse/validate layer only	Stores Impl::return_type, but nothing consumes it for derivation

Supporting checks:

• Compound signature keys are unchanged. Markers are stripped when building them, so or:bool stays or:bool — asserted directly in substrait-go's extensions/extension_mgr_test.go.
• No library fixture encodes the affected declarations. The only boolean? occurrences across the four are type-parser and derivation-expression unit tests on literal strings (substrait-go types/parser/parse_test.go, substrait-python tests/test_derivation_expression.py).
• It propagates as data, not code. substrait-java vendors the spec as a submodule. The Rust path goes through the substrait-extensions crate, which copies the extension YAMLs verbatim and exposes them as include_str! constants; its generated Rust types come from the text/*.yaml schemas, which this PR does not touch, so there is no generated-code diff.

All four were inspected at commits level with their respective upstream main.

One observable difference, for the record

substrait-rs's parsed Impl::return_type.nullable does flip to false for these 7 impls. Because substrait-rs has no MIRROR derivation, a consumer reading return_type directly as a call's output type would move from "always nullable" (wrong when all arguments are non-nullable) to "always required" (wrong when an argument is nullable). Neither is spec-correct without implementing the derivation, so this is not a regression the policy would act on — but it is the one place where the parsed value changes, and I would rather flag it here than have it turn up later.

🤖 Generated with AI