PHOENIX-7982 Data corruption in UPSERT SELECT with VARBINARY_ENCODED

phoenix-core-client/src/main/java/org/apache/phoenix/compile/UpsertCompiler.java

@@ -112,6 +112,7 @@
 import org.apache.phoenix.schema.types.PTimestamp;
 import org.apache.phoenix.schema.types.PUnsignedLong;
 import org.apache.phoenix.schema.types.PVarbinary;
+import org.apache.phoenix.schema.types.PVarbinaryEncoded;
 import org.apache.phoenix.util.ByteUtil;
 import org.apache.phoenix.util.ExpressionUtil;
 import org.apache.phoenix.util.IndexUtil;
@@ -271,9 +272,16 @@ public static MutationState upsertSelect(StatementContext childContext, TableRef
             throw new DataExceedsCapacityException(column.getDataType(), column.getMaxLength(),
               column.getScale(), column.getName().getString());
           }
-          column.getDataType().coerceBytes(ptr, value, column.getDataType(), precision, scale,
-            SortOrder.getDefault(), column.getMaxLength(), column.getScale(), column.getSortOrder(),
-            table.rowKeyOrderOptimizable());
+          if (column.getDataType() == PVarbinaryEncoded.INSTANCE) {
+            // ptr holds the decoded value, because that is what the result set hands back for
+            // VARBINARY_ENCODED. coerceBytes() cannot restore the encoding here: it treats all
+            // binary types as byte comparable and so leaves ptr untouched (PHOENIX-7982).
+            ptr.set(column.getDataType().toBytes(value, column.getSortOrder()));
+          } else {
+            column.getDataType().coerceBytes(ptr, value, column.getDataType(), precision, scale,
+              SortOrder.getDefault(), column.getMaxLength(), column.getScale(),
+              column.getSortOrder(), table.rowKeyOrderOptimizable());
+          }
           values[j] = ByteUtil.copyKeyBytesIfNecessary(ptr);
         }
         setValues(values, pkSlotIndexes, columnIndexes, table, mutation, statement,

phoenix-core-client/src/main/java/org/apache/phoenix/expression/CoerceExpression.java

@@ -27,10 +27,7 @@
 import org.apache.phoenix.expression.visitor.ExpressionVisitor;
 import org.apache.phoenix.schema.SortOrder;
 import org.apache.phoenix.schema.tuple.Tuple;
-import org.apache.phoenix.schema.types.PBinary;
 import org.apache.phoenix.schema.types.PDataType;
-import org.apache.phoenix.schema.types.PVarbinary;
-import org.apache.phoenix.schema.types.PVarbinaryEncoded;
 
 import org.apache.phoenix.thirdparty.com.google.common.base.Preconditions;
 import org.apache.phoenix.thirdparty.com.google.common.collect.ImmutableList;
@@ -164,35 +161,10 @@ public boolean isStateless() {
 
   @Override
   public boolean evaluate(Tuple tuple, ImmutableBytesWritable ptr) {
-    // For CoerceExpression evaluation, lhs is coerced to rhs literal expression. However,
-    // in case of variable length binary literal expression, literal value by default
-    // gets VARBINARY data type. If lhs expression is of type VARBINARY_ENCODED, we should
-    // encode rhs literal value to VARBINARY_ENCODED type. This makes the eventual coerce
-    // evaluation successful.
-    if (
-      getChild() instanceof LiteralExpression
-        && (getChild().getDataType() == PVarbinary.INSTANCE
-          || getChild().getDataType() == PBinary.INSTANCE)
-        && getDataType() == PVarbinaryEncoded.INSTANCE
-    ) {
-      Expression expression;
-      try {
-        expression = LiteralExpression.newConstant(((LiteralExpression) getChild()).getValue(),
-          PVarbinaryEncoded.INSTANCE);
-      } catch (SQLException e) {
-        throw new RuntimeException(e);
-      }
-      if (expression.evaluate(tuple, ptr)) {
-        getDataType().coerceBytes(ptr, null, expression.getDataType(), expression.getMaxLength(),
-          null, expression.getSortOrder(), maxLength, null, getSortOrder(), rowKeyOrderOptimizable);
-        return true;
-      }
-    } else {
-      if (getChild().evaluate(tuple, ptr)) {
-        getDataType().coerceBytes(ptr, null, getChild().getDataType(), getChild().getMaxLength(),
-          null, getChild().getSortOrder(), maxLength, null, getSortOrder(), rowKeyOrderOptimizable);
-        return true;
-      }
+    if (getChild().evaluate(tuple, ptr)) {
+      getDataType().coerceBytes(ptr, null, getChild().getDataType(), getChild().getMaxLength(),
+        null, getChild().getSortOrder(), maxLength, null, getSortOrder(), rowKeyOrderOptimizable);
+      return true;
     }
     return false;
   }

phoenix-core-client/src/main/java/org/apache/phoenix/schema/types/PBinary.java

@@ -124,6 +124,9 @@ public Object toObject(byte[] bytes, int offset, int length, PDataType actualTyp
 
   @Override
   public Object toObject(Object object, PDataType actualType) {
+    if (actualType == PVarbinaryEncoded.INSTANCE) {
+      return object;
+    }
     return actualType.toBytes(object);
   }
 

phoenix-core-client/src/main/java/org/apache/phoenix/schema/types/PDataType.java

@@ -91,8 +91,10 @@ public final PDataCodec getCodec() {
   }
 
   public boolean isBytesComparableWith(PDataType otherType) {
-    return equalsAny(this, otherType, PVarbinary.INSTANCE, PBinary.INSTANCE,
-      PVarbinaryEncoded.INSTANCE);
+    if (this == PVarbinaryEncoded.INSTANCE || otherType == PVarbinaryEncoded.INSTANCE) {
+      return this.equals(otherType);
+    }
+    return equalsAny(this, otherType, PVarbinary.INSTANCE, PBinary.INSTANCE);
   }
 
   /** Returns true if {@link PDataType} can be declared as primary key otherwise false. */

phoenix-core-client/src/main/java/org/apache/phoenix/schema/types/PVarbinary.java

@@ -88,6 +88,9 @@ public Object toObject(byte[] bytes, int offset, int length, PDataType actualTyp
 
   @Override
   public Object toObject(Object object, PDataType actualType) {
+    if (actualType == PVarbinaryEncoded.INSTANCE) {
+      return object;
+    }
     return actualType.toBytes(object);
   }
 
@@ -121,7 +124,7 @@ public int compareTo(Object lhs, Object rhs, PDataType rhsType) {
     } else if (rhs == null) {
       return 1;
     }
-    if (equalsAny(rhsType, this, PBinary.INSTANCE)) {
+    if (equalsAny(rhsType, this, PBinary.INSTANCE, PVarbinaryEncoded.INSTANCE)) {
       return Bytes.compareTo((byte[]) lhs, (byte[]) rhs);
     } else {
       byte[] rhsBytes = rhsType.toBytes(rhs);