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 2023/07/25 22:47:12 UTC

[GitHub] [iceberg] rdblue opened a new pull request, #8149: API: Implement bound expression sanitization

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

   This implements sanitization for bound expressions and adds an option to bind an expression when sanitizing. The purpose is to get better sanitization for dates and timestamps, which may show up as longs when pushed down from Spark.


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


[GitHub] [iceberg] rdblue merged pull request #8149: API: Implement bound expression sanitization

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


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


[GitHub] [iceberg] nastra commented on a diff in pull request #8149: API: Implement bound expression sanitization

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


##########
api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java:
##########
@@ -176,6 +223,28 @@ public static String describe(Term term) {
     }
   }
 
+  public static <T> UnboundTerm<T> unbind(BoundTerm<T> term) {
+    if (term instanceof BoundTransform) {
+      BoundTransform<?, T> bound = (BoundTransform<?, T>) term;
+      return Expressions.transform(bound.ref().name(), bound.transform());
+    } else if (term instanceof BoundReference) {
+      return Expressions.ref(((BoundReference<T>) term).name());
+    }
+
+    throw new UnsupportedOperationException("Cannot unbind unsupported term: " + term);
+  }
+
+  @SuppressWarnings("unchecked")
+  public static <T> UnboundTerm<T> unbind(Term term) {

Review Comment:
   this seems to be unused, should we remove the 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


[GitHub] [iceberg] rdblue commented on pull request #8149: API: Implement bound expression sanitization

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

   Thanks for reviewing, @nastra!


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


[GitHub] [iceberg] rdblue commented on a diff in pull request #8149: API: Implement bound expression sanitization

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


##########
api/src/main/java/org/apache/iceberg/expressions/ExpressionUtil.java:
##########
@@ -176,6 +223,28 @@ public static String describe(Term term) {
     }
   }
 
+  public static <T> UnboundTerm<T> unbind(BoundTerm<T> term) {
+    if (term instanceof BoundTransform) {
+      BoundTransform<?, T> bound = (BoundTransform<?, T>) term;
+      return Expressions.transform(bound.ref().name(), bound.transform());
+    } else if (term instanceof BoundReference) {
+      return Expressions.ref(((BoundReference<T>) term).name());
+    }
+
+    throw new UnsupportedOperationException("Cannot unbind unsupported term: " + term);
+  }
+
+  @SuppressWarnings("unchecked")
+  public static <T> UnboundTerm<T> unbind(Term term) {

Review Comment:
   I think it's useful so I'd probably keep it. Since we are adding the ability to unbind terms, it seems like doing both versions at once is the right thing to do.



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