huaxingao commented on code in PR #15632:
URL: https://github.com/apache/iceberg/pull/15632#discussion_r2968272831
##########
api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java:
##########
@@ -145,21 +146,26 @@ public static String toSanitizedString(
}
/**
- * Extracts an expression that references only the given column IDs from the
given expression.
+ * Returns an expression that retains only predicates which reference one of
the given field IDs.
*
- * <p>The result is inclusive. If a row would match the original filter, it
must match the result
- * filter.
- *
- * @param expression a filter Expression
- * @param schema a Schema
+ * @param expression a filter expression
+ * @param schema schema for binding references
* @param caseSensitive whether binding is case sensitive
- * @param ids field IDs used to match predicates to extract from the
expression
- * @return an Expression that selects at least the same rows as the original
using only the IDs
+ * @param ids field IDs to retain predicates for
+ * @return expression containing only predicates that reference the given IDs
*/
public static Expression extractByIdInclusive(
Expression expression, Schema schema, boolean caseSensitive, int... ids)
{
- PartitionSpec spec = identitySpec(schema, ids);
- return Projections.inclusive(spec,
caseSensitive).project(Expressions.rewriteNot(expression));
+ if (ids == null || ids.length == 0) {
+ return Expressions.alwaysTrue();
+ }
Review Comment:
nit: shall we put an empty line after control block?
##########
api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java:
##########
@@ -145,21 +146,26 @@ public static String toSanitizedString(
}
/**
- * Extracts an expression that references only the given column IDs from the
given expression.
+ * Returns an expression that retains only predicates which reference one of
the given field IDs.
*
- * <p>The result is inclusive. If a row would match the original filter, it
must match the result
- * filter.
- *
- * @param expression a filter Expression
- * @param schema a Schema
+ * @param expression a filter expression
+ * @param schema schema for binding references
* @param caseSensitive whether binding is case sensitive
- * @param ids field IDs used to match predicates to extract from the
expression
- * @return an Expression that selects at least the same rows as the original
using only the IDs
+ * @param ids field IDs to retain predicates for
+ * @return expression containing only predicates that reference the given IDs
*/
public static Expression extractByIdInclusive(
Expression expression, Schema schema, boolean caseSensitive, int... ids)
{
- PartitionSpec spec = identitySpec(schema, ids);
- return Projections.inclusive(spec,
caseSensitive).project(Expressions.rewriteNot(expression));
+ if (ids == null || ids.length == 0) {
+ return Expressions.alwaysTrue();
+ }
+ ImmutableSet.Builder<Integer> retainIds = ImmutableSet.builder();
+ for (int id : ids) {
+ retainIds.add(id);
+ }
Review Comment:
ditto
--
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]
