xuzifu666 commented on PR #5386: URL: https://github.com/apache/calcite/pull/5386#issuecomment-6660144771
> I'd drop `Parameter.nullable()` and read nullness from type-use annotations only, specifically jspecify. Nullness is a property of the parameter type, so the type system is the right place for it. An attribute on `@Parameter` duplicates it, can contradict it (`@Nullable @Parameter(nullable = false)`), cannot describe nested types such as `List<@Nullable String>`, and adds a public API to linq4j that we would have to keep. > > Proposed rule: honor jspecify `@Nullable` and `@NonNull` only. An unannotated reference type is non-null only inside null-marked scope: the innermost `@NullMarked` or `@NullUnmarked` on the method, enclosing classes, package, or module decides. Elsewhere it keeps today's nullable default, so UDFs without jspecify annotations keep their types. Primitives stay `NOT NULL`. > > The PR should also say what a non-null parameter means at the SQL level. Today a `NULL` argument for a parameter whose type is `NOT NULL` is accepted and the call returns `NULL`, so the declared type is not enforced. Either the validator rejects such a call, or the description states that only the reported parameter type changes. Thanks for the review @vlsi and all points addressed, PTAL~ -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
