roryqi commented on code in PR #14237:
URL: https://github.com/apache/gravitino/pull/14237#discussion_r4399026901


##########
iceberg/iceberg-rest-server-experiment/src/main/java/org/apache/gravitino/iceberg/service/restriction/SqlRestrictionLiterals.java:
##########
@@ -0,0 +1,199 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.iceberg.service.restriction;
+
+import java.math.BigDecimal;
+import java.math.RoundingMode;
+import java.time.DateTimeException;
+import java.time.LocalDate;
+import java.time.LocalDateTime;
+import java.time.LocalTime;
+import java.time.OffsetDateTime;
+import java.time.ZoneOffset;
+import java.time.temporal.ChronoUnit;
+import javax.annotation.Nullable;
+import org.apache.gravitino.policy.sql.Literal;
+import org.apache.gravitino.policy.sql.LiteralType;
+import org.apache.gravitino.policy.sql.SqlType;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
+
+/** Exact logical-type and literal binding shared by SQL predicates and masks. 
*/
+final class SqlRestrictionLiterals {
+  private SqlRestrictionLiterals() {}
+
+  static Type type(SqlType declared) {
+    switch (declared.name()) {
+      case "BOOLEAN":
+        return Types.BooleanType.get();
+      case "INTEGER":
+        return Types.IntegerType.get();
+      case "LONG":
+        return Types.LongType.get();
+      case "FLOAT":
+        return Types.FloatType.get();
+      case "DOUBLE":
+        return Types.DoubleType.get();
+      case "STRING":
+        return Types.StringType.get();
+      case "BINARY":
+        return Types.BinaryType.get();
+      case "DATE":
+        return Types.DateType.get();
+      case "DECIMAL":
+        return Types.DecimalType.of(declared.precision(), declared.scale());
+      case "TIME":
+        temporalPrecision(declared);
+        return Types.TimeType.get();
+      case "TIMESTAMP":
+        temporalPrecision(declared);
+        return declared.withTimeZone()
+            ? Types.TimestampType.withZone()
+            : Types.TimestampType.withoutZone();
+      default:
+        throw new IllegalArgumentException("Unsupported declared SQL type: " + 
declared);
+    }
+  }
+
+  @Nullable
+  static Object value(Literal source, Type expected) {
+    if (source.type() != null && !type(source.type()).equals(expected)) {
+      throw new IllegalArgumentException("Literal or context type does not 
match " + expected);
+    }
+    if (source.literalType() == LiteralType.NULL) {
+      return null;
+    }
+    try {
+      switch (expected.typeId()) {
+        case BOOLEAN:
+          kind(source, LiteralType.BOOLEAN);
+          if (!source.value().equalsIgnoreCase("TRUE")
+              && !source.value().equalsIgnoreCase("FALSE")) {
+            throw new IllegalArgumentException("Invalid Boolean literal");
+          }
+          return Boolean.parseBoolean(source.value());
+        case STRING:
+          kind(source, LiteralType.STRING);
+          return source.value();
+        case INTEGER:
+          kind(source, LiteralType.NUMBER);
+          return new BigDecimal(source.value()).intValueExact();
+        case LONG:
+          kind(source, LiteralType.NUMBER);
+          return new BigDecimal(source.value()).longValueExact();
+        case FLOAT:
+          kind(source, LiteralType.NUMBER);
+          BigDecimal floatDecimal = new BigDecimal(source.value());
+          float floatValue = Float.parseFloat(source.value());
+          if (!Float.isFinite(floatValue)
+              || new BigDecimal((double) floatValue).compareTo(floatDecimal) 
!= 0) {
+            throw new IllegalArgumentException("Inexact FLOAT literal");
+          }
+          return floatValue;
+        case DOUBLE:
+          kind(source, LiteralType.NUMBER);
+          BigDecimal doubleDecimal = new BigDecimal(source.value());
+          double doubleValue = Double.parseDouble(source.value());
+          if (!Double.isFinite(doubleValue)
+              || new BigDecimal(doubleValue).compareTo(doubleDecimal) != 0) {
+            throw new IllegalArgumentException("Inexact DOUBLE literal");
+          }
+          return doubleValue;
+        case DECIMAL:
+          kind(source, LiteralType.NUMBER);
+          Types.DecimalType decimalType = (Types.DecimalType) expected;
+          BigDecimal decimal =
+              new BigDecimal(source.value())
+                  .setScale(decimalType.scale(), RoundingMode.UNNECESSARY);
+          if (decimal.precision() > decimalType.precision()) {
+            throw new IllegalArgumentException("Decimal precision overflow");
+          }
+          return decimal;
+        case DATE:
+          kind(source, LiteralType.TEMPORAL);
+          return Math.toIntExact(LocalDate.parse(source.value()).toEpochDay());
+        case TIME:
+          kind(source, LiteralType.TEMPORAL);
+          LocalTime time = LocalTime.parse(source.value());
+          if (!time.equals(time.truncatedTo(ChronoUnit.MICROS))) {
+            throw new IllegalArgumentException("Unsupported temporal 
precision");
+          }
+          return time.toNanoOfDay() / 1000;
+        case TIMESTAMP:
+          kind(source, LiteralType.TEMPORAL);
+          String text = source.value().replace(' ', 'T');
+          boolean zoned = ((Types.TimestampType) expected).shouldAdjustToUTC();
+          LocalDateTime timestamp =
+              zoned
+                  ? OffsetDateTime.parse(text)
+                      .withOffsetSameInstant(ZoneOffset.UTC)
+                      .toLocalDateTime()
+                  : LocalDateTime.parse(text);
+          if (!timestamp.equals(timestamp.truncatedTo(ChronoUnit.MICROS))) {
+            throw new IllegalArgumentException("Unsupported temporal 
precision");
+          }
+          return ChronoUnit.MICROS.between(LocalDateTime.of(1970, 1, 1, 0, 0), 
timestamp);
+        default:
+          throw new IllegalArgumentException("Unsupported literal target: " + 
expected);
+      }
+    } catch (ArithmeticException | DateTimeException exception) {
+      throw new IllegalArgumentException(
+          "Literal cannot be represented exactly as " + expected, exception);
+    }
+  }
+
+  static String canonicalValue(Object value) {
+    if (value instanceof Float) {
+      return Integer.toUnsignedString(Float.floatToRawIntBits((Float) value));
+    }
+    if (value instanceof Double) {
+      return Long.toUnsignedString(Double.doubleToRawLongBits((Double) value));
+    }
+    if (value instanceof BigDecimal) {
+      return ((BigDecimal) value).toPlainString();
+    }
+    return value.toString();
+  }
+
+  @SuppressWarnings("unchecked")
+  static int compare(Object left, Object right) {
+    if (left instanceof BigDecimal) {
+      return ((BigDecimal) left).compareTo((BigDecimal) right);
+    }
+    if (left instanceof Float && (Float) left == 0F && (Float) right == 0F) {
+      return 0;
+    }
+    if (left instanceof Double && (Double) left == 0D && (Double) right == 0D) 
{
+      return 0;
+    }
+    return ((Comparable<Object>) left).compareTo(right);

Review Comment:
   Fixed in 392b1888. Constant/context string comparisons now use Iceberg 
Comparators.charSequences(), matching bound STRING predicates; the ordering 
contract is documented. Added regressions for U+E000 versus U+10000 across 
comparisons, CASE, BETWEEN, and context-based mask selection. Thanks for 
catching this!



##########
iceberg/iceberg-rest-server-experiment/src/main/java/org/apache/gravitino/iceberg/service/restriction/SqlRestrictionMasks.java:
##########
@@ -0,0 +1,182 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *  http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.gravitino.iceberg.service.restriction;
+
+import java.math.BigDecimal;
+import java.util.Objects;
+import javax.annotation.Nullable;
+import org.apache.gravitino.policy.sql.CaseBranch;
+import org.apache.gravitino.policy.sql.CaseExpression;
+import org.apache.gravitino.policy.sql.FunctionCall;
+import org.apache.gravitino.policy.sql.InputReference;
+import org.apache.gravitino.policy.sql.Literal;
+import org.apache.gravitino.policy.sql.LiteralType;
+import org.apache.gravitino.policy.sql.SqlExpression;
+import org.apache.gravitino.policy.sql.SqlFunction;
+import org.apache.iceberg.expressions.Expression;
+import org.apache.iceberg.functions.IcebergFunction;
+import org.apache.iceberg.functions.IcebergFunctions;
+import org.apache.iceberg.types.Types;
+
+/** Converts selected scalar SQL mask results to one supported Iceberg 
projection per field. */
+final class SqlRestrictionMasks {
+  private final Types.NestedField field;
+  private final SqlRestrictionPredicates predicates;
+
+  SqlRestrictionMasks(Types.NestedField field, SqlRestrictionPredicates 
predicates) {
+    this.field = field;
+    this.predicates = predicates;
+  }
+
+  @Nullable
+  IcebergFunction<?, ?> resolve(SqlExpression body) {
+    if (field.isRequired() && nullable(body)) {
+      throw new IllegalArgumentException(
+          "Nullable SQL mask result is invalid for required field " + 
field.fieldId());
+    }
+    return selected(body);
+  }
+
+  @Nullable
+  private IcebergFunction<?, ?> selected(SqlExpression body) {
+    if (body instanceof InputReference) {
+      return null;
+    }
+    if (body instanceof CaseExpression) {
+      CaseExpression choice = (CaseExpression) body;
+      for (CaseBranch branch : choice.branches()) {
+        SqlPredicateResult condition =
+            choice.selector() == null
+                ? predicates.resolve(branch.condition())
+                : equality(choice.selector(), branch.condition());
+        if (!condition.isConstant()) {
+          throw SqlRestrictionBinding.unsupported("row-dependent mask 
selection");
+        }
+        if (condition.trueExpression().op() == Expression.Operation.TRUE) {
+          return selected(branch.result());
+        }
+      }
+      return choice.fallback() == null
+          ? checked(IcebergFunctions.replaceWithNull(field.fieldId()))
+          : selected(choice.fallback());
+    }
+    if (body instanceof FunctionCall) {
+      FunctionCall call = (FunctionCall) body;
+      if (call.arguments().size() == 1 && call.arguments().get(0) instanceof 
InputReference) {
+        return checked(mask(call.function()));
+      }
+    }
+    Literal literal = predicates.constantScalar(body);
+    Object value = SqlRestrictionLiterals.value(literal, field.type());
+    if (value == null) {
+      return checked(IcebergFunctions.replaceWithNull(field.fieldId()));
+    }
+    IcebergFunction<?, ?> fixed = 
checked(IcebergFunctions.maskToFixedValue(field.fieldId()));
+    Object fixedValue = fixed.bind(field.type()).apply(null);
+    boolean same =
+        value instanceof BigDecimal && fixedValue instanceof BigDecimal
+            ? ((BigDecimal) value).compareTo((BigDecimal) fixedValue) == 0
+            : Objects.equals(value, fixedValue);
+    if (!same) {
+      throw SqlRestrictionBinding.unsupported("an arbitrary constant mask 
value");
+    }
+    return fixed;
+  }
+
+  private SqlPredicateResult equality(SqlExpression selector, SqlExpression 
match) {
+    return predicates.equal(selector, match);
+  }
+
+  private IcebergFunction<?, ?> mask(SqlFunction function) {
+    switch (function) {
+      case MASK_ALPHANUM:
+        return IcebergFunctions.maskAlphanum(field.fieldId());
+      case SHOW_FIRST_4:
+        return IcebergFunctions.showFirst4(field.fieldId());
+      case SHOW_LAST_4:
+        return IcebergFunctions.showLast4(field.fieldId());
+      case MASK_TO_FIXED_VALUE:
+        return IcebergFunctions.maskToFixedValue(field.fieldId());
+      case REPLACE_WITH_NULL:
+        return IcebergFunctions.replaceWithNull(field.fieldId());
+      case TRUNCATE_TO_YEAR:
+        return IcebergFunctions.truncateToYear(field.fieldId());
+      case TRUNCATE_TO_MONTH:
+        return IcebergFunctions.truncateToMonth(field.fieldId());
+      case SHA256_GLOBAL:
+        return IcebergFunctions.sha256Global(field.fieldId());
+      case SHA256_QUERY_LOCAL:
+        return IcebergFunctions.sha256QueryLocal(field.fieldId());
+      default:
+        throw SqlRestrictionBinding.unsupported("the selected mask function " 
+ function);
+    }
+  }
+
+  private IcebergFunction<?, ?> checked(IcebergFunction<?, ?> function) {
+    if (function.name().equals("replace-with-null") && field.isRequired()) {
+      throw new IllegalArgumentException(
+          "NULL mask is invalid for required field " + field.fieldId());
+    }
+    if (!function.canBind(field.type())) {
+      throw new IllegalArgumentException(
+          "Mask " + function.name() + " is incompatible with selected field 
type " + field.type());
+    }
+    return function;
+  }
+
+  private boolean nullable(SqlExpression expression) {
+    if (expression instanceof Literal) {
+      return ((Literal) expression).literalType() == LiteralType.NULL;
+    }
+    if (expression instanceof InputReference) {
+      return field.isOptional();
+    }
+    if (expression instanceof CaseExpression) {
+      CaseExpression choice = (CaseExpression) expression;
+      if (choice.fallback() == null || nullable(choice.fallback())) {
+        return true;
+      }
+      for (CaseBranch branch : choice.branches()) {
+        if (nullable(branch.result())) {
+          return true;
+        }
+      }
+      return false;
+    }
+    if (expression instanceof FunctionCall) {
+      FunctionCall call = (FunctionCall) expression;
+      switch (call.function()) {
+        case CURRENT_USER:
+        case IS_GROUP_MEMBER:
+        case MASK_TO_FIXED_VALUE:
+          return false;
+        case REPLACE_WITH_NULL:
+          return true;
+        default:
+          for (SqlExpression argument : call.arguments()) {
+            if (nullable(argument)) {
+              return true;
+            }
+          }
+          return false;
+      }
+    }
+    return true;

Review Comment:
   Fixed in 392b1888. IS NULL / IS NOT NULL are now non-nullable, and Boolean 
operations propagate nullability from their operands. Added regressions for 
required BOOLEAN masks, including value IS NULL resolving to the fixed FALSE 
projection, nullable results being rejected, and unsupported projections still 
failing closed. Thanks!



-- 
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]

Reply via email to