You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by "rdblue (via GitHub)" <gi...@apache.org> on 2025/02/18 17:03:46 UTC

[PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

rdblue opened a new pull request, #12311:
URL: https://github.com/apache/iceberg/pull/12311

   This updates `InclusiveMetricsEvaluator` that uses column stats to skip data files during scan planning.
   
   The evaluator was implementing the older `BoundExpressionVisitor` interface that only supported `BoundReference` and not other `BoundTerm` instances like `BoundTransform`. After #12304, `BoundExtract` also needs to be supported.
   
   Filtering works for transformed values when the transform is order preserving. If it is not order preserving (like `bucket`) the bounds cannot be used.
   
   Most of the changes to `BoundExpressionVisitor` are to produce the lower and upper bounds values that are tested:
   * For `BoundReference`, deserialize the bound from the correct `lowerBounds` or `upperBounds` map (moved to `parseLowerBound` and `parseLowerBound`
   * For `BoundTransform`, deserialize the bound and transform it if the transform is order-preserving (in `transformLowerBound` and `transformUpperBound`)
   * For `Extract`, deserialize the bound as a map from field name to `VariantValue`, then convert the value to the internal representation (in `extractLowerBound` and `extractUpperBound`)
   
   Note that this implementation **temporarily** uses Java serialization to convert the Variant field bounds to bytes. Once we have defined a serialization for these values, this should use it instead. Serialization and parsing is handled by `VariantDataUtil`.
   
   This adds new test suites for the new `BoundTerm` cases that are supported:
   * `TestInclusiveMetricsEvaluatorWithExtract` tests variant cases
   * `TestInclusiveMetricsEvaluatorWithTransforms` tests transform cases


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#discussion_r1970487043


##########
api/src/main/java/org/apache/iceberg/variants/VariantUtil.java:
##########
@@ -18,21 +18,112 @@
  */
 package org.apache.iceberg.variants;
 
+import java.math.BigDecimal;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.nio.charset.StandardCharsets;
+import java.util.Map;
 import java.util.function.Function;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
 
-class VariantUtil {
+public class VariantUtil {
   private static final int BASIC_TYPE_MASK = 0b11;
   private static final int BASIC_TYPE_PRIMITIVE = 0;
   private static final int BASIC_TYPE_SHORT_STRING = 1;
   private static final int BASIC_TYPE_OBJECT = 2;
   private static final int BASIC_TYPE_ARRAY = 3;
 
+  // TODO: Implement PhysicalType.TIME
+  // TODO: Implement PhysicalType.TIMESTAMPNTZ_NANO and PhysicalType.TIMESTAMPTZ_NANO
+  // TODO: Implement PhysicalType.UUID
+  private static final Map<Type, PhysicalType> NO_CONVERSION_NEEDED =
+      ImmutableMap.<Type, PhysicalType>builder()
+          .put(Types.IntegerType.get(), PhysicalType.INT32)
+          .put(Types.LongType.get(), PhysicalType.INT64)
+          .put(Types.FloatType.get(), PhysicalType.FLOAT)
+          .put(Types.DoubleType.get(), PhysicalType.DOUBLE)
+          .put(Types.DateType.get(), PhysicalType.DATE)
+          .put(Types.TimestampType.withoutZone(), PhysicalType.TIMESTAMPNTZ)
+          .put(Types.TimestampType.withZone(), PhysicalType.TIMESTAMPTZ)
+          .put(Types.StringType.get(), PhysicalType.STRING)
+          .put(Types.BinaryType.get(), PhysicalType.BINARY)
+          .put(Types.UnknownType.get(), PhysicalType.NULL)
+          .build();
+
   private VariantUtil() {}
 
+  @SuppressWarnings("unchecked")
+  public static <T> T castTo(VariantValue value, Type type) {

Review Comment:
   I don't think that we should expose this on `VariantValue` and in fact I almost moved it into `InclusiveMetricsEvaluator` because the behavior here is a bit misleading. The problem is that we don't want to make callers think that this conversion is equivalent to any SQL operation because SQL engines will have a lot more complex logic.
   
   For instance, a SQL engine might have an expression like `CAST(extract(var_col, "$.field") AS INT) < 10`. Iceberg does something similar, but the cast expression in SQL is much more general and may handle cases like variant string to int. We want to avoid a false equivalence with SQL behavior, which is up to the engine. This is why the `Extract.eval` method is not implemented.
   
   I think the right thing is actually to make this less public and move it into a util class in expressions so that those classes can use it, but we don't expose it more broadly.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "danielcweeks (via GitHub)" <gi...@apache.org>.
danielcweeks commented on code in PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#discussion_r1970288476


##########
api/src/main/java/org/apache/iceberg/variants/VariantUtil.java:
##########
@@ -18,21 +18,112 @@
  */
 package org.apache.iceberg.variants;
 
+import java.math.BigDecimal;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.nio.charset.StandardCharsets;
+import java.util.Map;
 import java.util.function.Function;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
 
-class VariantUtil {
+public class VariantUtil {
   private static final int BASIC_TYPE_MASK = 0b11;
   private static final int BASIC_TYPE_PRIMITIVE = 0;
   private static final int BASIC_TYPE_SHORT_STRING = 1;
   private static final int BASIC_TYPE_OBJECT = 2;
   private static final int BASIC_TYPE_ARRAY = 3;
 
+  // TODO: Implement PhysicalType.TIME
+  // TODO: Implement PhysicalType.TIMESTAMPNTZ_NANO and PhysicalType.TIMESTAMPTZ_NANO
+  // TODO: Implement PhysicalType.UUID
+  private static final Map<Type, PhysicalType> NO_CONVERSION_NEEDED =
+      ImmutableMap.<Type, PhysicalType>builder()
+          .put(Types.IntegerType.get(), PhysicalType.INT32)
+          .put(Types.LongType.get(), PhysicalType.INT64)
+          .put(Types.FloatType.get(), PhysicalType.FLOAT)
+          .put(Types.DoubleType.get(), PhysicalType.DOUBLE)
+          .put(Types.DateType.get(), PhysicalType.DATE)
+          .put(Types.TimestampType.withoutZone(), PhysicalType.TIMESTAMPNTZ)
+          .put(Types.TimestampType.withZone(), PhysicalType.TIMESTAMPTZ)
+          .put(Types.StringType.get(), PhysicalType.STRING)
+          .put(Types.BinaryType.get(), PhysicalType.BINARY)
+          .put(Types.UnknownType.get(), PhysicalType.NULL)
+          .build();
+
   private VariantUtil() {}
 
+  @SuppressWarnings("unchecked")
+  public static <T> T castTo(VariantValue value, Type type) {

Review Comment:
   Wouldn't this make more sense as a static method on `VariantValue`?  That would allow us to keep the the util class package protected and this feels more closely aligned with `VariantValue` anyway.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#discussion_r1960172147


##########
core/src/test/java/org/apache/iceberg/expressions/TestInclusiveMetricsEvaluatorWithExtract.java:
##########
@@ -0,0 +1,687 @@
+/*
+ * 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.expressions;
+
+import static org.apache.iceberg.expressions.Expressions.and;
+import static org.apache.iceberg.expressions.Expressions.equal;
+import static org.apache.iceberg.expressions.Expressions.extract;
+import static org.apache.iceberg.expressions.Expressions.greaterThan;
+import static org.apache.iceberg.expressions.Expressions.greaterThanOrEqual;
+import static org.apache.iceberg.expressions.Expressions.in;
+import static org.apache.iceberg.expressions.Expressions.isNaN;
+import static org.apache.iceberg.expressions.Expressions.isNull;
+import static org.apache.iceberg.expressions.Expressions.lessThan;
+import static org.apache.iceberg.expressions.Expressions.lessThanOrEqual;
+import static org.apache.iceberg.expressions.Expressions.not;
+import static org.apache.iceberg.expressions.Expressions.notEqual;
+import static org.apache.iceberg.expressions.Expressions.notIn;
+import static org.apache.iceberg.expressions.Expressions.notNaN;
+import static org.apache.iceberg.expressions.Expressions.notNull;
+import static org.apache.iceberg.expressions.Expressions.notStartsWith;
+import static org.apache.iceberg.expressions.Expressions.or;
+import static org.apache.iceberg.expressions.Expressions.startsWith;
+import static org.apache.iceberg.types.Types.NestedField.optional;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.assertj.core.api.Assertions.assertThatThrownBy;
+
+import java.util.List;
+import java.util.Map;
+import org.apache.iceberg.DataFile;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.TestHelpers.Row;
+import org.apache.iceberg.TestHelpers.TestDataFile;
+import org.apache.iceberg.exceptions.ValidationException;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.relocated.com.google.common.collect.Lists;
+import org.apache.iceberg.types.Types;
+import org.apache.iceberg.types.Types.IntegerType;
+import org.apache.iceberg.variants.PhysicalType;
+import org.apache.iceberg.variants.VariantDataUtil;
+import org.apache.iceberg.variants.Variants;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.FieldSource;
+
+public class TestInclusiveMetricsEvaluatorWithExtract {
+  private static final Schema SCHEMA =
+      new Schema(
+          required(1, "id", IntegerType.get()),
+          required(2, "variant", Types.VariantType.get()),
+          optional(3, "all_nulls", Types.VariantType.get()));
+
+  private static final int INT_MIN_VALUE = 30;
+  private static final int INT_MAX_VALUE = 79;
+
+  private static final DataFile FILE =
+      new TestDataFile(
+          "file.avro",
+          Row.of(),
+          50,
+          // any value counts, including nulls
+          ImmutableMap.<Integer, Long>builder().put(1, 50L).put(2, 50L).put(3, 50L).buildOrThrow(),
+          // null value counts
+          ImmutableMap.<Integer, Long>builder()
+              .put(1, 0L)
+              .put(2, 0L)
+              .put(3, 50L) // all_nulls
+              .buildOrThrow(),
+          // nan value counts
+          null,
+          // lower bounds
+          ImmutableMap.of(
+              2,
+              VariantDataUtil.serializeBounds(
+                  Map.of(
+                      "event_id",
+                      Variants.of(PhysicalType.INT32, INT_MIN_VALUE),
+                      "str",
+                      Variants.of(PhysicalType.STRING, "abc")))),
+          // upper bounds
+          ImmutableMap.of(
+              2,
+              VariantDataUtil.serializeBounds(
+                  Map.of(
+                      "event_id",
+                      Variants.of(PhysicalType.INT32, INT_MAX_VALUE),
+                      "str",
+                      Variants.of(PhysicalType.STRING, "abe")))));
+
+  private boolean shouldRead(Expression expr) {
+    return shouldRead(expr, FILE);
+  }
+
+  private boolean shouldRead(Expression expr, DataFile file) {
+    return shouldRead(expr, file, true);
+  }
+
+  private boolean shouldReadCaseInsensitive(Expression expr) {
+    return shouldRead(expr, FILE, false);
+  }
+
+  private boolean shouldRead(Expression expr, DataFile file, boolean caseSensitive) {
+    return new InclusiveMetricsEvaluator(SCHEMA, expr, caseSensitive).eval(file);
+  }
+
+  @Test
+  public void testAllNulls() {
+    assertThat(shouldRead(isNull(extract("all_nulls", "$.event_id", "long"))))
+        .as("Should read: null values exist in all null column")
+        .isTrue();
+
+    assertThat(shouldRead(notNull(extract("all_nulls", "$.event_id", "long"))))
+        .as("Should skip: no non-null value in all null column")
+        .isFalse();
+
+    assertThat(shouldRead(lessThan(extract("all_nulls", "$.event_id", "long"), 30)))
+        .as("Should skip: lessThan on all null column")
+        .isFalse();
+
+    assertThat(shouldRead(lessThanOrEqual(extract("all_nulls", "$.event_id", "long"), 30)))
+        .as("Should skip: lessThanOrEqual on all null column")
+        .isFalse();
+
+    assertThat(shouldRead(greaterThan(extract("all_nulls", "$.event_id", "long"), 30)))
+        .as("Should skip: greaterThan on all null column")
+        .isFalse();
+
+    assertThat(shouldRead(greaterThanOrEqual(extract("all_nulls", "$.event_id", "long"), 30)))
+        .as("Should skip: greaterThanOrEqual on all null column")
+        .isFalse();
+
+    assertThat(shouldRead(equal(extract("all_nulls", "$.event_id", "long"), 30)))
+        .as("Should skip: equal on all null column")
+        .isFalse();
+
+    assertThat(shouldRead(notEqual(extract("all_nulls", "$.event_id", "long"), 30)))
+        .as("Should read: notEqual on all null column")
+        .isTrue();
+
+    assertThat(shouldRead(in(extract("all_nulls", "$.event_type", "string"), "abc", "def")))
+        .as("Should skip: in on all nulls column")
+        .isFalse();
+
+    assertThat(shouldRead(notIn(extract("all_nulls", "$.event_type", "string"), "abc", "def")))
+        .as("Should read: notIn on all nulls column")
+        .isTrue();
+
+    assertThat(shouldRead(startsWith(extract("all_nulls", "$.event_type", "string"), "a")))
+        .as("Should skip: startsWith on all null column")
+        .isFalse();
+
+    assertThat(shouldRead(notStartsWith(extract("all_nulls", "$.event_type", "string"), "a")))
+        .as("Should read: notStartsWith on all null column")
+        .isTrue();
+
+    assertThat(shouldRead(isNaN(extract("all_nulls", "$.measurement", "double"))))
+        .as("Should skip: no NaN value in all null column")
+        .isFalse();
+
+    assertThat(shouldRead(notNaN(extract("all_nulls", "$.measurement", "double"))))
+        .as("Should read: all null column has non-NaN values")
+        .isTrue();
+  }
+
+  @Test
+  public void testIsNaNAndNotNaN() {
+    assertThat(shouldRead(isNaN(extract("variant", "$.measurement", "double"))))
+        .as("Should read: variant may contain NaN")
+        .isTrue();
+
+    assertThat(shouldRead(notNaN(extract("variant", "$.measurement", "double"))))
+        .as("Should read: variant may contain non-NaN")
+        .isTrue();
+  }
+
+  @Test
+  public void testMissingColumn() {
+    assertThatThrownBy(() -> shouldRead(lessThan(extract("missing", "$", "int"), 5)))
+        .isInstanceOf(ValidationException.class)
+        .hasMessageContaining("Cannot find field 'missing'");
+  }
+
+  private static final Expression[] MISSING_STATS_EXPRESSIONS =
+      new Expression[] {
+        lessThan(extract("variant", "$.missing", "long"), 5),
+        lessThanOrEqual(extract("variant", "$.missing", "long"), 30),
+        equal(extract("variant", "$.missing", "long"), 70),
+        greaterThan(extract("variant", "$.missing", "long"), 78),
+        greaterThanOrEqual(extract("variant", "$.missing", "long"), 90),
+        notEqual(extract("variant", "$.missing", "long"), 101),
+        isNull(extract("variant", "$.missing", "string")),
+        notNull(extract("variant", "$.missing", "string")),
+        isNaN(extract("variant", "$.missing", "float")),
+        notNaN(extract("variant", "$.missing", "float"))
+      };
+
+  @ParameterizedTest
+  @FieldSource("MISSING_STATS_EXPRESSIONS")
+  public void testMissingStats(Expression expr) {

Review Comment:
   If the bounds are in metadata, they are trusted. Bounds should be missing in cases where there are variant values with incompatible types.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue merged PR #12311:
URL: https://github.com/apache/iceberg/pull/12311


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#discussion_r1970492212


##########
api/src/main/java/org/apache/iceberg/variants/VariantUtil.java:
##########
@@ -18,21 +18,112 @@
  */
 package org.apache.iceberg.variants;
 
+import java.math.BigDecimal;
 import java.nio.ByteBuffer;
 import java.nio.ByteOrder;
 import java.nio.charset.StandardCharsets;
+import java.util.Map;
 import java.util.function.Function;
 import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
 
-class VariantUtil {
+public class VariantUtil {
   private static final int BASIC_TYPE_MASK = 0b11;
   private static final int BASIC_TYPE_PRIMITIVE = 0;
   private static final int BASIC_TYPE_SHORT_STRING = 1;
   private static final int BASIC_TYPE_OBJECT = 2;
   private static final int BASIC_TYPE_ARRAY = 3;
 
+  // TODO: Implement PhysicalType.TIME
+  // TODO: Implement PhysicalType.TIMESTAMPNTZ_NANO and PhysicalType.TIMESTAMPTZ_NANO
+  // TODO: Implement PhysicalType.UUID
+  private static final Map<Type, PhysicalType> NO_CONVERSION_NEEDED =
+      ImmutableMap.<Type, PhysicalType>builder()
+          .put(Types.IntegerType.get(), PhysicalType.INT32)
+          .put(Types.LongType.get(), PhysicalType.INT64)
+          .put(Types.FloatType.get(), PhysicalType.FLOAT)
+          .put(Types.DoubleType.get(), PhysicalType.DOUBLE)
+          .put(Types.DateType.get(), PhysicalType.DATE)
+          .put(Types.TimestampType.withoutZone(), PhysicalType.TIMESTAMPNTZ)
+          .put(Types.TimestampType.withZone(), PhysicalType.TIMESTAMPTZ)
+          .put(Types.StringType.get(), PhysicalType.STRING)
+          .put(Types.BinaryType.get(), PhysicalType.BINARY)
+          .put(Types.UnknownType.get(), PhysicalType.NULL)
+          .build();
+
   private VariantUtil() {}
 
+  @SuppressWarnings("unchecked")
+  public static <T> T castTo(VariantValue value, Type type) {

Review Comment:
   I moved this so that it is now package-private within expressions.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#issuecomment-2675600178

   This now relies on #12374 to move the serialized variant classes into API so that `InclusiveMetricsEvaluator` can use them for deserializing bounds.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#issuecomment-2675317539

   This needed changes to `TestSparkScan` because adding support for transforms filters out additional files in unpartitioned tables.


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on code in PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#discussion_r1960173362


##########
api/src/main/java/org/apache/iceberg/variants/VariantDataUtil.java:
##########
@@ -0,0 +1,189 @@
+/*
+ * 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.variants;
+
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.IOException;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
+import java.math.BigDecimal;
+import java.nio.ByteBuffer;
+import java.util.List;
+import java.util.Map;
+import org.apache.iceberg.exceptions.RuntimeIOException;
+import org.apache.iceberg.relocated.com.google.common.base.Preconditions;
+import org.apache.iceberg.relocated.com.google.common.base.Splitter;
+import org.apache.iceberg.relocated.com.google.common.collect.ImmutableMap;
+import org.apache.iceberg.types.Type;
+import org.apache.iceberg.types.Types;
+
+public class VariantDataUtil {
+  private VariantDataUtil() {}
+
+  private static final Splitter DOT = Splitter.on(".");
+  private static final String ROOT = "$";
+
+  /**
+   * Extracts a nested value from nested variant objects, or returns null of no value matches.
+   *
+   * @param value a variant value
+   * @param fields a list of field names to extract
+   * @return the nested value identified by the fields
+   */
+  public static VariantValue extract(VariantValue value, List<String> fields) {
+    VariantValue current = value;
+    for (String field : fields) {
+      if (current != null && current.type() == PhysicalType.OBJECT) {
+        current = current.asObject().get(field);
+      } else {
+        return null;
+      }
+    }
+
+    return current;
+  }
+
+  public static ByteBuffer serializeBounds(Map<String, VariantValue> bounds) {

Review Comment:
   This serialization is temporary and will be replaced with a well-defined variant serialization.



-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org


Re: [PR] API, Core: Update inclusive metrics evaluator for extract and transforms [iceberg]

Posted by "rdblue (via GitHub)" <gi...@apache.org>.
rdblue commented on PR #12311:
URL: https://github.com/apache/iceberg/pull/12311#issuecomment-2683615946

   Thanks for reviewing, @danielcweeks!


-- 
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: issues-unsubscribe@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@iceberg.apache.org
For additional commands, e-mail: issues-help@iceberg.apache.org