yashmayya commented on code in PR #13573: URL: https://github.com/apache/pinot/pull/13573#discussion_r1674147033
########## pinot-common/src/main/java/org/apache/pinot/common/function/scalar/LogicalFunctions.java: ########## @@ -18,13 +18,11 @@ */ package org.apache.pinot.common.function.scalar; -import org.apache.calcite.linq4j.function.Strict; import org.apache.pinot.spi.annotations.ScalarFunction; /** * Logical transformation on boolean values. Currently, only not is supported. */ -@Strict Review Comment: Earlier when we were registering our scalar functions with Calcite [here](https://github.com/apache/pinot/blob/fdfae5e57aaeb4cd8ca2921cee33c2a45534e40c/pinot-common/src/main/java/org/apache/pinot/common/function/FunctionRegistry.java#L135) we were relying on the return type inference [here](https://github.com/apache/calcite/blob/694b556a2ece4953d8e9145352eb2340e1fac908/core/src/main/java/org/apache/calcite/schema/impl/ScalarFunctionImpl.java#L158) which created a non-nullable type for Pinot's scalar methods with primitive return types. The `@Strict` and `@SemiStrict` annotations were added in https://github.com/apache/pinot/pull/13255 because of the way "null intolerant" functions in Pinot are handled (wherein even primitive returning scalar functions could return `null` when invoked with null operands). However, Jackie's changes here take care of that - https://github.com/apache/pinot/pull/13573/files#diff-5e0c97ffa4b3dac15512f091d1e38f66b0ff6140c487576a57ba652c9f71b051R247-R264 so we no longer need these Calcite annotations. -- 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
