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

[PR] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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

   ParquetFilters.ConvertFilterToParquet performs conversion from an Iceberg filter (Expression) to a Parquet FilterPredicate. It currently only handles IS_NULL, NOT_NULL, IS_NAN, NOT_NAN, EQ, NOT_EQ, GT, GT_EQ, LT, LT_EQ. It does not handle, e.g., IN and NOT_IN, which would be useful to have.
   
   This conversion can be used for setting a record filter in Parquet and then using that to filter row groups when reading Parquet files.


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetFilters.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.parquet;
+
+import static org.apache.iceberg.expressions.Expressions.in;
+import static org.apache.iceberg.expressions.Expressions.notIn;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.types.Types.DoubleType;
+import org.apache.iceberg.types.Types.IntegerType;
+import org.apache.parquet.column.ParquetProperties.WriterVersion;
+import org.apache.parquet.filter2.compat.FilterCompat;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestParquetFilters {
+
+  private static final java.util.Map<String, Integer> ALIASES =
+      new java.util.HashMap<String, Integer>() {
+        {
+          put("id", 1);
+          put("age", 2);
+        }
+      };
+
+  private static final Schema SCHEMA =
+      new Schema(
+          ALIASES, required(1, "id", IntegerType.get()), required(2, "age", DoubleType.get()));
+
+  @Parameters(name = "writerVersion={0}")
+  public static Collection<WriterVersion> parameters() {
+    return Arrays.asList(WriterVersion.PARQUET_1_0, WriterVersion.PARQUET_2_0);
+  }
+
+  @TestTemplate
+  public void testIntegerInFilter() {
+    FilterCompat.Filter filter =
+        (FilterCompat.Filter) ParquetFilters.convert(SCHEMA, in("id", 1, 2, 3), true);
+
+    try {
+      java.lang.reflect.Field privateField = filter.getClass().getDeclaredField("filterPredicate");
+      privateField.setAccessible(true);
+      assertThat(privateField.get(filter).toString().equalsIgnoreCase("in(id, 1, 2, 3)")).isTrue();

Review Comment:
   Extracted it to a seperate method.



-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -204,6 +204,10 @@ public Schema(int schemaId, NestedField... columns) {
     this(schemaId, Arrays.asList(columns));
   }
 
+  public Schema(Map<String, Integer> aliases, NestedField... columns) {
+    this(Arrays.asList(columns), aliases);
+  }

Review Comment:
   I removed it in the latest version.



-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
api/src/main/java/org/apache/iceberg/Schema.java:
##########
@@ -204,6 +204,10 @@ public Schema(int schemaId, NestedField... columns) {
     this(schemaId, Arrays.asList(columns));
   }
 
+  public Schema(Map<String, Integer> aliases, NestedField... columns) {
+    this(Arrays.asList(columns), aliases);
+  }

Review Comment:
   I'd be hesitant to add to the API. This is only used for a test; can't you use an existing constructor?



-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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

   > @sririshindra thanks for this PR. Do you think it is worth adding tests where the column is a long/timestamp and where the column is a string (binary column)?
   
   Thanks @wypoon I added a test for long and string as well. That actually turned out to be very useful as that caught a bug that I didn't know previously. There was an unnecessary cast of the litset to HashSet in the previous version. I now removed it and the Tests that I added all succeed now.  


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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

   Bump! This feature would simplify the query-generation logic in a client-app.


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #12449: Support In and notIn operators in ParquetFilters.ConvertFilterToParquet
URL: https://github.com/apache/iceberg/pull/12449


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetFilters.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.parquet;
+
+import static org.apache.iceberg.expressions.Expressions.in;
+import static org.apache.iceberg.expressions.Expressions.notIn;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.types.Types.DoubleType;
+import org.apache.iceberg.types.Types.IntegerType;
+import org.apache.parquet.column.ParquetProperties.WriterVersion;
+import org.apache.parquet.filter2.compat.FilterCompat;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestParquetFilters {
+
+  private static final java.util.Map<String, Integer> ALIASES =
+      new java.util.HashMap<String, Integer>() {
+        {
+          put("id", 1);
+          put("age", 2);
+        }
+      };

Review Comment:
   Fixed in latest commit.



-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetFilters.java:
##########
@@ -118,11 +119,14 @@ public <T> FilterPredicate predicate(BoundPredicate<T> pred) {
       Operation op = pred.op();
       BoundReference<T> ref = (BoundReference<T>) pred.term();
       String path = schema.idToAlias(ref.fieldId());
-      Literal<T> lit;
+      Literal<T> lit = null;
+      Set<T> litSet = null;
       if (pred.isUnaryPredicate()) {
         lit = null;
       } else if (pred.isLiteralPredicate()) {
         lit = pred.asLiteralPredicate().literal();
+      } else if (pred.isSetPredicate()) {
+        litSet = pred.asSetPredicate().asSetPredicate().literalSet();

Review Comment:
   Fixed in latest commit.



-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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

   @rdblue can you review this?
   I have some stale PRs implementing Parquet page skipping and at that time I came across the fact that `ParquetFilters.ConvertFilterToParquet` does not convert `IN`, for example.


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetFilters.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.parquet;
+
+import static org.apache.iceberg.expressions.Expressions.in;
+import static org.apache.iceberg.expressions.Expressions.notIn;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.types.Types.DoubleType;
+import org.apache.iceberg.types.Types.IntegerType;
+import org.apache.parquet.column.ParquetProperties.WriterVersion;
+import org.apache.parquet.filter2.compat.FilterCompat;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestParquetFilters {
+
+  private static final java.util.Map<String, Integer> ALIASES =
+      new java.util.HashMap<String, Integer>() {
+        {
+          put("id", 1);
+          put("age", 2);
+        }
+      };
+
+  private static final Schema SCHEMA =
+      new Schema(
+          ALIASES, required(1, "id", IntegerType.get()), required(2, "age", DoubleType.get()));
+
+  @Parameters(name = "writerVersion={0}")
+  public static Collection<WriterVersion> parameters() {
+    return Arrays.asList(WriterVersion.PARQUET_1_0, WriterVersion.PARQUET_2_0);
+  }
+
+  @TestTemplate
+  public void testIntegerInFilter() {
+    FilterCompat.Filter filter =
+        (FilterCompat.Filter) ParquetFilters.convert(SCHEMA, in("id", 1, 2, 3), true);
+
+    try {
+      java.lang.reflect.Field privateField = filter.getClass().getDeclaredField("filterPredicate");
+      privateField.setAccessible(true);
+      assertThat(privateField.get(filter).toString().equalsIgnoreCase("in(id, 1, 2, 3)")).isTrue();

Review Comment:
   Since you do this more than once, you can consider a helper method to get the private `Field`, make it accessible, and then get the filter from it and doing a `toString()` on that.



##########
parquet/src/test/java/org/apache/iceberg/parquet/TestParquetFilters.java:
##########
@@ -0,0 +1,86 @@
+/*
+ * 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.parquet;
+
+import static org.apache.iceberg.expressions.Expressions.in;
+import static org.apache.iceberg.expressions.Expressions.notIn;
+import static org.apache.iceberg.types.Types.NestedField.required;
+import static org.assertj.core.api.Assertions.assertThat;
+
+import java.util.Arrays;
+import java.util.Collection;
+import org.apache.iceberg.ParameterizedTestExtension;
+import org.apache.iceberg.Parameters;
+import org.apache.iceberg.Schema;
+import org.apache.iceberg.types.Types.DoubleType;
+import org.apache.iceberg.types.Types.IntegerType;
+import org.apache.parquet.column.ParquetProperties.WriterVersion;
+import org.apache.parquet.filter2.compat.FilterCompat;
+import org.junit.jupiter.api.TestTemplate;
+import org.junit.jupiter.api.extension.ExtendWith;
+
+@ExtendWith(ParameterizedTestExtension.class)
+public class TestParquetFilters {
+
+  private static final java.util.Map<String, Integer> ALIASES =
+      new java.util.HashMap<String, Integer>() {
+        {
+          put("id", 1);
+          put("age", 2);
+        }
+      };

Review Comment:
   In Iceberg, the preference is to avoid instantiating something like `java.util.HashMap` or `java.util.ArrayList` directly. You can use `com.google.common.collect.ImmutableMap` here instead. `ImmutableMap.of("id", 1, "age", 2)` would work.



-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12449:
URL: https://github.com/apache/iceberg/pull/12449#issuecomment-2920867099

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12449:
URL: https://github.com/apache/iceberg/pull/12449#issuecomment-2951278066

   This pull request has been closed due to lack of activity. This is not a judgement on the merit of the PR in any way. It is just a way of keeping the PR queue manageable. If you think that is incorrect, or the pull request requires review, you can revive the PR at any time.


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #12449:
URL: https://github.com/apache/iceberg/pull/12449#issuecomment-2829110819

   This pull request has been marked as stale due to 30 days of inactivity. It will be closed in 1 week if no further activity occurs. If you think that’s incorrect or this pull request requires a review, please simply write any comment. If closed, you can revive the PR at any time and @mention a reviewer or discuss it on the dev@iceberg.apache.org list. Thank you for your contributions.


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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

   I originally created https://github.com/apache/iceberg/pull/10341 . It did not get traction at the time and it was closed. I would appreciate if the committers can take another look at this as we think its a legitimate improvement. cc: @wypoon 


-- 
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] Support In and notIn operators in ParquetFilters.ConvertFilterToParquet [iceberg]

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


##########
parquet/src/main/java/org/apache/iceberg/parquet/ParquetFilters.java:
##########
@@ -118,11 +119,14 @@ public <T> FilterPredicate predicate(BoundPredicate<T> pred) {
       Operation op = pred.op();
       BoundReference<T> ref = (BoundReference<T>) pred.term();
       String path = schema.idToAlias(ref.fieldId());
-      Literal<T> lit;
+      Literal<T> lit = null;
+      Set<T> litSet = null;
       if (pred.isUnaryPredicate()) {
         lit = null;
       } else if (pred.isLiteralPredicate()) {
         lit = pred.asLiteralPredicate().literal();
+      } else if (pred.isSetPredicate()) {
+        litSet = pred.asSetPredicate().asSetPredicate().literalSet();

Review Comment:
   ```
           litSet = pred.asSetPredicate().literalSet();
   ```
   (You call `asSetPredicate()` twice.)



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