sungwy commented on code in PR #16198: URL: https://github.com/apache/iceberg/pull/16198#discussion_r4136641169
########## api/src/main/java/org/apache/iceberg/functions/MaskAlphanum.java: ########## @@ -0,0 +1,84 @@ +/* + * 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.iceberg.functions; + +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.types.Type; +import org.apache.iceberg.util.SerializableFunction; + +/** Redacts every Unicode code point in a string per the mask-alphanum rules. */ +public final class MaskAlphanum extends BaseFunction<String, String> { + static final String NAME = "mask-alphanum"; Review Comment: is our goal to make these masking actions available as Iceberg Functions that can be referenced in Iceberg Expressions? If so, should use snakecase in the name instead so that it's SQL addressable without quotes? https://github.com/apache/iceberg/blob/main/format/expressions-spec.md#apply-function ########## api/src/main/java/org/apache/iceberg/functions/BaseFunction.java: ########## @@ -0,0 +1,58 @@ +/* + * 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.iceberg.functions; + +import java.util.Objects; + +/** Base for all concrete {@link IcebergFunction} implementations; holds the field id. */ +abstract class BaseFunction<S, T> implements IcebergFunction<S, T> { + private final int fieldId; + + BaseFunction(int fieldId) { + this.fieldId = fieldId; + } + + @Override + public final int fieldId() { Review Comment: Does the `fieldId` have to be on the `BaseFunction`/`IcebergFunction`? Nothing in the class uses it apart from equals/hashCode; every bind works only from the Type. I see field related information as being important for running validation checks (nested field consistency check, and required/optional check), but that's not something the integer `fieldId` brings into the class. Would it be better to remove fieldId and leave those checks at a higher order class that calls `SerializableFunction<S, T> bind(Type type)` and make this class fieldId-agnostic? That matches how `PartitionField` pairs a source id with a `Transform`, with validation in `PartitionSpec.checkCompatibility`. Unless there's a reason why we want `equals` and `hashCode` to depend on it ########## api/src/main/java/org/apache/iceberg/functions/MaskToFixedValue.java: ########## @@ -0,0 +1,213 @@ +/* + * 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.iceberg.functions; + +import java.io.Serializable; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.nio.ByteBuffer; +import java.nio.ByteOrder; +import java.time.LocalDate; +import java.time.LocalDateTime; +import java.util.Collections; +import java.util.List; +import java.util.UUID; +import org.apache.iceberg.StructLike; +import org.apache.iceberg.relocated.com.google.common.base.Preconditions; +import org.apache.iceberg.types.Type; +import org.apache.iceberg.types.Types; +import org.apache.iceberg.util.DateTimeUtil; +import org.apache.iceberg.util.SerializableFunction; +import org.apache.iceberg.variants.Variant; + +/** Returns a spec-defined fixed value for the column's type. */ +public final class MaskToFixedValue extends BaseFunction<Object, Object> { + + private static final Integer INT_DEFAULT = 0; + private static final Long LONG_DEFAULT = 0L; + private static final Float FLOAT_DEFAULT = 0.0f; + private static final Double DOUBLE_DEFAULT = 0.0d; + // Per spec: string uses "XXXXXXXX" (not "") so masked strings stay visually distinct from + // legitimately empty strings. All other types use the zero value of their representation. + private static final String STRING_DEFAULT = "XXXXXXXX"; + private static final Integer DATE_DEFAULT = DateTimeUtil.daysFromDate(LocalDate.of(1970, 1, 1)); + private static final Long TIME_DEFAULT_MICROS = 0L; + private static final Long TIMESTAMP_DEFAULT_MICROS = + DateTimeUtil.microsFromTimestamp(LocalDateTime.of(1970, 1, 1, 0, 0)); + private static final Long TIMESTAMP_DEFAULT_NANOS = + DateTimeUtil.nanosFromTimestamp(LocalDateTime.of(1970, 1, 1, 0, 0)); + private static final UUID UUID_DEFAULT = UUID.fromString("00000000-0000-0000-0000-000000000000"); + private static final ByteBuffer EMPTY_BUFFER = ByteBuffer.allocate(0).asReadOnlyBuffer(); + + // Empty Variant: V1 metadata with no entries + empty object value. + private static final Variant EMPTY_VARIANT = + Variant.from( + ByteBuffer.wrap(new byte[] {0x01, 0x00, 0x00, 0x02, 0x00, 0x00}) + .order(ByteOrder.LITTLE_ENDIAN)); + + static final String NAME = "mask-to-fixed-value"; + + MaskToFixedValue(int fieldId) { + super(fieldId); + } + + @Override + public String name() { + return NAME; + } + + @Override + public boolean canBind(Type type) { + switch (type.typeId()) { + case BOOLEAN: + case INTEGER: + case LONG: + case FLOAT: + case DOUBLE: + case STRING: + case DATE: + case TIME: + case TIMESTAMP: + case TIMESTAMP_NANO: + case UUID: + case FIXED: + case BINARY: + case DECIMAL: + case VARIANT: + case LIST: + case MAP: + case STRUCT: Review Comment: for STRUCT: don't we need to check if each of its descendants can be bound to MaskToFixedValue? I'm thinking of a struct type that has Geography and Geometry types as its descendants. That'll unfortunately pass this check and then fail when converting to defaultValue. -- 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]
