You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@iceberg.apache.org by GitBox <gi...@apache.org> on 2022/08/18 22:59:16 UTC

[GitHub] [iceberg] kbendick opened a new pull request, #5574: Spark 3.2: Align formatting in bucket and truncate functions

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

   This PR aligns formatting in TruncateFunction and BucketFunction in Spark 3.2.
   
   It backports https://github.com/apache/iceberg/pull/5573 to Spark 3.2


-- 
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] aokolnychyi commented on a diff in pull request #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#discussion_r949675029


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/functions/BucketFunction.java:
##########
@@ -205,7 +211,6 @@ public static Integer invoke(int numBuckets, UTF8String value) {
         return null;
       }
 
-      // TODO - We can probably hash the bytes directly given they're already UTF-8 input.

Review Comment:
   I'd say it is a valid TODO item. We actually have a similar optimization internally.



-- 
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] kbendick commented on a diff in pull request #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#discussion_r949709302


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/functions/BucketFunction.java:
##########
@@ -205,7 +211,6 @@ public static Integer invoke(int numBuckets, UTF8String value) {
         return null;
       }
 
-      // TODO - We can probably hash the bytes directly given they're already UTF-8 input.

Review Comment:
   You're right. We left the optimization in truncate but didn't move forward with it for `bucket`. My mistake.



-- 
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 #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
rdblue commented on code in PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#discussion_r950332646


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/functions/BucketFunction.java:
##########
@@ -205,7 +211,6 @@ public static Integer invoke(int numBuckets, UTF8String value) {
         return null;
       }
 
-      // TODO - We can probably hash the bytes directly given they're already UTF-8 input.

Review Comment:
   Yeah, we should be able to pass the backing byte array. Let's do this in a follow-up for 3.3.



-- 
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] aokolnychyi commented on a diff in pull request #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#discussion_r949674800


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/functions/BucketFunction.java:
##########
@@ -226,9 +231,11 @@ public String canonicalName() {
 
     @Override
     public Integer produceResult(InternalRow input) {
-      return input.isNullAt(NUM_BUCKETS_ORDINAL) || input.isNullAt(VALUE_ORDINAL)
-          ? null
-          : invoke(input.getInt(NUM_BUCKETS_ORDINAL), input.getUTF8String(VALUE_ORDINAL));
+      if (input.isNullAt(NUM_BUCKETS_ORDINAL) || input.isNullAt(VALUE_ORDINAL)) {
+        return null;
+      } else {
+        return invoke(input.getInt(NUM_BUCKETS_ORDINAL), input.getInt(VALUE_ORDINAL));

Review Comment:
   Looks like a typo. Shall it still be `input.getUTF8String()`?



-- 
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] aokolnychyi commented on a diff in pull request #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
aokolnychyi commented on code in PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#discussion_r949675029


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/functions/BucketFunction.java:
##########
@@ -205,7 +211,6 @@ public static Integer invoke(int numBuckets, UTF8String value) {
         return null;
       }
 
-      // TODO - We can probably hash the bytes directly given they're already UTF-8 input.

Review Comment:
   I'd say it is a valid TODO item. We actually have a similar optimization internally.
   I think we still have this TODO in 3.3.



-- 
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] kbendick commented on pull request #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
kbendick commented on PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#issuecomment-1220058560

   cc @aokolnychyi 


-- 
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 #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
rdblue merged PR #5574:
URL: https://github.com/apache/iceberg/pull/5574


-- 
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] kbendick commented on a diff in pull request #5574: Spark 3.2: Align formatting in bucket and truncate functions

Posted by GitBox <gi...@apache.org>.
kbendick commented on code in PR #5574:
URL: https://github.com/apache/iceberg/pull/5574#discussion_r949710130


##########
spark/v3.2/spark/src/main/java/org/apache/iceberg/spark/functions/BucketFunction.java:
##########
@@ -226,9 +231,11 @@ public String canonicalName() {
 
     @Override
     public Integer produceResult(InternalRow input) {
-      return input.isNullAt(NUM_BUCKETS_ORDINAL) || input.isNullAt(VALUE_ORDINAL)
-          ? null
-          : invoke(input.getInt(NUM_BUCKETS_ORDINAL), input.getUTF8String(VALUE_ORDINAL));
+      if (input.isNullAt(NUM_BUCKETS_ORDINAL) || input.isNullAt(VALUE_ORDINAL)) {
+        return null;
+      } else {
+        return invoke(input.getInt(NUM_BUCKETS_ORDINAL), input.getInt(VALUE_ORDINAL));

Review Comment:
   Good catch. Updated!



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