Revisit handling IS_NULL conditions on right source table columns in LEFT_OUTER virtual tables (#3217)

Author: brfrn169

core/src/main/java/com/scalar/db/common/CoreError.java

@@ -1009,6 +1009,12 @@ public enum CoreError implements ScalarDbError {
       "Source tables cannot be dropped while virtual tables depending on them exist. Source table: %s; Virtual tables: %s",
       "",
       ""),
+  DELETE_IF_IS_NULL_FOR_RIGHT_SOURCE_TABLE_NOT_ALLOWED_FOR_LEFT_OUTER_VIRTUAL_TABLES(
+      Category.USER_ERROR,
+      "0276",
+      "The DeleteIf IS_NULL condition for right source table columns is not allowed in LEFT_OUTER virtual tables. Virtual table: %s",
+      "",
+      ""),
 
   //
   // Errors for the concurrency error category

core/src/main/java/com/scalar/db/storage/jdbc/JdbcDatabase.java

@@ -35,6 +35,7 @@
 import com.scalar.db.exception.storage.NoMutationException;
 import com.scalar.db.exception.storage.RetriableExecutionException;
 import com.scalar.db.io.Column;
+import com.scalar.db.util.ScalarDbUtils;
 import java.sql.Connection;
 import java.sql.SQLException;
 import java.util.ArrayList;
@@ -395,7 +396,19 @@ private List<Put> dividePutForSourceTables(Put put, VirtualTableInfo virtualTabl
           putBuilderForLeftSourceTable.condition(ConditionBuilder.putIf(leftExpressions));
         }
         if (!rightExpressions.isEmpty()) {
-          putBuilderForRightSourceTable.condition(ConditionBuilder.putIf(rightExpressions));
+          if (isAllIsNullOnRightColumnsInLeftOuterJoin(virtualTableInfo, rightExpressions)
+              && JdbcOperationAttributes
+                  .isLeftOuterVirtualTablePutIfIsNullOnRightColumnsConversionEnabled(put)) {
+            // In a LEFT_OUTER join, when all conditions on the right source table columns are
+            // IS_NULL, we cannot distinguish whether we should check for the existence of a
+            // right-side record with NULL values or for the case where the right-side record does
+            // not exist at all. Therefore, this behavior is controlled by the operation attribute.
+            // By default, we convert the condition to PutIfNotExists, assuming that the more common
+            // use case is to check that the right-side record does not exist.
+            putBuilderForRightSourceTable.condition(ConditionBuilder.putIfNotExists());
+          } else {
+            putBuilderForRightSourceTable.condition(ConditionBuilder.putIf(rightExpressions));
+          }
         }
       }
     }
@@ -464,7 +477,25 @@ private List<Delete> divideDeleteForSourceTables(Delete delete, VirtualTableInfo
           deleteBuilderForLeftSourceTable.condition(ConditionBuilder.deleteIf(leftExpressions));
         }
         if (!rightExpressions.isEmpty()) {
-          deleteBuilderForRightSourceTable.condition(ConditionBuilder.deleteIf(rightExpressions));
+          if (isAllIsNullOnRightColumnsInLeftOuterJoin(virtualTableInfo, rightExpressions)
+              && !JdbcOperationAttributes
+                  .isLeftOuterVirtualTableDeleteIfIsNullOnRightColumnsAllowed(delete)) {
+            // In a LEFT_OUTER join, when all conditions on the right source table columns are
+            // IS_NULL, we cannot distinguish whether we should check for the existence of a
+            // right-side record with NULL values or for the case where the right-side record does
+            // not exist at all. This makes the delete operation semantically ambiguous. Therefore,
+            // this behavior is controlled by the operation attribute. By default, we disallow this
+            // operation to prevent unintended behavior.
+            assert delete.forNamespace().isPresent() && delete.forTable().isPresent();
+            throw new IllegalArgumentException(
+                CoreError
+                    .DELETE_IF_IS_NULL_FOR_RIGHT_SOURCE_TABLE_NOT_ALLOWED_FOR_LEFT_OUTER_VIRTUAL_TABLES
+                    .buildMessage(
+                        ScalarDbUtils.getFullTableName(
+                            delete.forNamespace().get(), delete.forTable().get())));
+          } else {
+            deleteBuilderForRightSourceTable.condition(ConditionBuilder.deleteIf(rightExpressions));
+          }
         }
       }
     }
@@ -474,6 +505,13 @@ private List<Delete> divideDeleteForSourceTables(Delete delete, VirtualTableInfo
     return Arrays.asList(deleteForLeftSourceTable, deleteForRightSourceTable);
   }
 
+  private boolean isAllIsNullOnRightColumnsInLeftOuterJoin(
+      VirtualTableInfo virtualTableInfo, List<ConditionalExpression> rightExpressions) {
+    return virtualTableInfo.getJoinType() == VirtualTableJoinType.LEFT_OUTER
+        && rightExpressions.stream()
+            .allMatch(e -> e.getOperator() == ConditionalExpression.Operator.IS_NULL);
+  }
+
   private void close(Connection connection) {
     try {
       if (connection != null) {

core/src/main/java/com/scalar/db/storage/jdbc/JdbcOperationAttributes.java

@@ -0,0 +1,49 @@
+package com.scalar.db.storage.jdbc;
+
+import com.scalar.db.api.Delete;
+import com.scalar.db.api.Put;
+import java.util.Map;
+
+/** A utility class to manipulate the operation attributes for JDBC. */
+public final class JdbcOperationAttributes {
+  private static final String OPERATION_ATTRIBUTE_PREFIX = "jdbc-";
+
+  public static final String
+      LEFT_OUTER_VIRTUAL_TABLE_PUT_IF_IS_NULL_ON_RIGHT_COLUMNS_CONVERSION_ENABLED =
+          OPERATION_ATTRIBUTE_PREFIX
+              + "left-outer-virtual-table-put-if-is-null-on-right-columns-conversion-enabled";
+
+  public static final String LEFT_OUTER_VIRTUAL_TABLE_DELETE_IF_IS_NULL_ON_RIGHT_COLUMNS_ALLOWED =
+      OPERATION_ATTRIBUTE_PREFIX
+          + "left-outer-virtual-table-delete-if-is-null-on-right-columns-allowed";
+
+  private JdbcOperationAttributes() {}
+
+  public static boolean isLeftOuterVirtualTablePutIfIsNullOnRightColumnsConversionEnabled(Put put) {
+    return put.getAttribute(
+            LEFT_OUTER_VIRTUAL_TABLE_PUT_IF_IS_NULL_ON_RIGHT_COLUMNS_CONVERSION_ENABLED)
+        .map(Boolean::parseBoolean)
+        .orElse(true);
+  }
+
+  public static void setLeftOuterVirtualTablePutIfIsNullOnRightColumnsConversionEnabled(
+      Map<String, String> attributes, boolean enabled) {
+    attributes.put(
+        LEFT_OUTER_VIRTUAL_TABLE_PUT_IF_IS_NULL_ON_RIGHT_COLUMNS_CONVERSION_ENABLED,
+        Boolean.toString(enabled));
+  }
+
+  public static boolean isLeftOuterVirtualTableDeleteIfIsNullOnRightColumnsAllowed(Delete delete) {
+    return delete
+        .getAttribute(LEFT_OUTER_VIRTUAL_TABLE_DELETE_IF_IS_NULL_ON_RIGHT_COLUMNS_ALLOWED)
+        .map(Boolean::parseBoolean)
+        .orElse(false);
+  }
+
+  public static void setLeftOuterVirtualTableDeleteIfIsNullOnRightColumnsAllowed(
+      Map<String, String> attributes, boolean allowed) {
+    attributes.put(
+        LEFT_OUTER_VIRTUAL_TABLE_DELETE_IF_IS_NULL_ON_RIGHT_COLUMNS_ALLOWED,
+        Boolean.toString(allowed));
+  }
+}