Copilot commented on code in PR #17189:
URL: https://github.com/apache/pinot/pull/17189#discussion_r2516257924
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/LogicalFunctions.java:
##########
@@ -18,20 +18,146 @@
*/
package org.apache.pinot.common.function.scalar;
+import javax.annotation.Nullable;
import org.apache.pinot.spi.annotations.ScalarFunction;
+
/**
- * Logical transformation on boolean values. Currently, only not is supported.
+ * Inbuilt logical transform functions with Trino-compatible NULL handling
+ *
+ * <p>These functions implement logical operators (AND, OR, NOT) with proper
three-valued logic
+ * support, following the SQL standard and Trino's behavior for NULL handling.
+ *
+ * <p>Truth tables for NULL handling:
+ * <ul>
+ * <li>AND: Returns NULL if one side is NULL and the other is not FALSE.
+ * Returns FALSE if at least one side is FALSE.</li>
+ * <li>OR: Returns NULL if one side is NULL and the other is not TRUE.
+ * Returns TRUE if at least one side is TRUE.</li>
+ * <li>NOT: Returns NULL if the input is NULL.</li>
+ * </ul>
+ *
+ * <p>Examples:
+ * <pre>
+ * logicalAnd(true, true) → true
+ * logicalAnd(true, false) → false
+ * logicalAnd(true, null) → null
+ * logicalAnd(false, null) → false
+ *
+ * logicalOr(true, false) → true
+ * logicalOr(false, false) → false
+ * logicalOr(false, null) → null
+ * logicalOr(true, null) → true
+ *
+ * logicalNot(true) → false
+ * logicalNot(false) → true
+ * logicalNot(null) → null
+ * </pre>
*/
public class LogicalFunctions {
+
private LogicalFunctions() {
}
/**
- * Returns logical negate of value
+ * Logical AND operator with Trino-compatible NULL handling.
+ *
+ * <p>Truth table:
+ * <pre>
+ * a | b | result
+ * ------|-------|-------
+ * TRUE | TRUE | TRUE
+ * TRUE | FALSE | FALSE
+ * TRUE | NULL | NULL
+ * FALSE | TRUE | FALSE
+ * FALSE | FALSE | FALSE
+ * FALSE | NULL | FALSE
+ * NULL | TRUE | NULL
+ * NULL | FALSE | FALSE
+ * NULL | NULL | NULL
+ * </pre>
+ *
+ * @param a First boolean value (nullable)
+ * @param b Second boolean value (nullable)
+ * @return The logical AND result, or NULL if result cannot be determined
+ */
+ @Nullable
+ @ScalarFunction(names = {"logicalAnd", "logical_and", "and"},
nullableParameters = true)
+ public static Boolean logicalAnd(@Nullable Boolean a, @Nullable Boolean b) {
+ // If either value is FALSE, the result is FALSE
+ if (Boolean.FALSE.equals(a) || Boolean.FALSE.equals(b)) {
Review Comment:
[nitpick] Using `Boolean.FALSE.equals()` is correct for null-safety, but for
performance-critical logical operations, consider restructuring to check nulls
first, then use primitive comparison. For example: `if ((a != null && !a) || (b
!= null && !b))` avoids boxing comparisons when both values are non-null.
```suggestion
if ((a != null && !a) || (b != null && !b)) {
```
##########
pinot-common/src/main/java/org/apache/pinot/common/function/scalar/LogicalFunctions.java:
##########
@@ -18,20 +18,146 @@
*/
package org.apache.pinot.common.function.scalar;
+import javax.annotation.Nullable;
import org.apache.pinot.spi.annotations.ScalarFunction;
+
/**
- * Logical transformation on boolean values. Currently, only not is supported.
+ * Inbuilt logical transform functions with Trino-compatible NULL handling
+ *
+ * <p>These functions implement logical operators (AND, OR, NOT) with proper
three-valued logic
+ * support, following the SQL standard and Trino's behavior for NULL handling.
+ *
+ * <p>Truth tables for NULL handling:
+ * <ul>
+ * <li>AND: Returns NULL if one side is NULL and the other is not FALSE.
+ * Returns FALSE if at least one side is FALSE.</li>
+ * <li>OR: Returns NULL if one side is NULL and the other is not TRUE.
+ * Returns TRUE if at least one side is TRUE.</li>
+ * <li>NOT: Returns NULL if the input is NULL.</li>
+ * </ul>
+ *
+ * <p>Examples:
+ * <pre>
+ * logicalAnd(true, true) → true
+ * logicalAnd(true, false) → false
+ * logicalAnd(true, null) → null
+ * logicalAnd(false, null) → false
+ *
+ * logicalOr(true, false) → true
+ * logicalOr(false, false) → false
+ * logicalOr(false, null) → null
+ * logicalOr(true, null) → true
+ *
+ * logicalNot(true) → false
+ * logicalNot(false) → true
+ * logicalNot(null) → null
+ * </pre>
*/
public class LogicalFunctions {
+
private LogicalFunctions() {
}
/**
- * Returns logical negate of value
+ * Logical AND operator with Trino-compatible NULL handling.
+ *
+ * <p>Truth table:
+ * <pre>
+ * a | b | result
+ * ------|-------|-------
+ * TRUE | TRUE | TRUE
+ * TRUE | FALSE | FALSE
+ * TRUE | NULL | NULL
+ * FALSE | TRUE | FALSE
+ * FALSE | FALSE | FALSE
+ * FALSE | NULL | FALSE
+ * NULL | TRUE | NULL
+ * NULL | FALSE | FALSE
+ * NULL | NULL | NULL
+ * </pre>
+ *
+ * @param a First boolean value (nullable)
+ * @param b Second boolean value (nullable)
+ * @return The logical AND result, or NULL if result cannot be determined
+ */
+ @Nullable
+ @ScalarFunction(names = {"logicalAnd", "logical_and", "and"},
nullableParameters = true)
+ public static Boolean logicalAnd(@Nullable Boolean a, @Nullable Boolean b) {
+ // If either value is FALSE, the result is FALSE
+ if (Boolean.FALSE.equals(a) || Boolean.FALSE.equals(b)) {
+ return false;
+ }
+
+ // If both values are TRUE, the result is TRUE
+ if (Boolean.TRUE.equals(a) && Boolean.TRUE.equals(b)) {
+ return true;
+ }
+
+ // Otherwise, at least one value is NULL and neither is FALSE, so result
is NULL
+ return null;
+ }
+
+ /**
+ * Logical OR operator with Trino-compatible NULL handling.
+ *
+ * <p>Truth table:
+ * <pre>
+ * a | b | result
+ * ------|-------|-------
+ * TRUE | TRUE | TRUE
+ * TRUE | FALSE | TRUE
+ * TRUE | NULL | TRUE
+ * FALSE | TRUE | TRUE
+ * FALSE | FALSE | FALSE
+ * FALSE | NULL | NULL
+ * NULL | TRUE | TRUE
+ * NULL | FALSE | NULL
+ * NULL | NULL | NULL
+ * </pre>
+ *
+ * @param a First boolean value (nullable)
+ * @param b Second boolean value (nullable)
+ * @return The logical OR result, or NULL if the result cannot be determined
+ */
+ @Nullable
+ @ScalarFunction(names = {"logicalOr", "logical_or", "or"},
nullableParameters = true)
+ public static Boolean logicalOr(@Nullable Boolean a, @Nullable Boolean b) {
+ // If either value is TRUE, the result is TRUE
+ if (Boolean.TRUE.equals(a) || Boolean.TRUE.equals(b)) {
Review Comment:
[nitpick] Similar to logicalAnd, using `Boolean.TRUE.equals()` is null-safe
but potentially less efficient. Consider checking nulls first: `if ((a != null
&& a) || (b != null && b))` for better performance with non-null values.
```suggestion
if ((a != null && a) || (b != null && b)) {
```
--
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]
