Skip to content

Conversation

@kbendick
Copy link
Contributor

This PR aligns formatting in TruncateFunction and BucketFunction in Spark 3.2.

It backports #5573 to Spark 3.2

@github-actions github-actions bot added the spark label Aug 18, 2022
@kbendick
Copy link
Contributor Author

cc @aokolnychyi

if (input.isNullAt(NUM_BUCKETS_ORDINAL) || input.isNullAt(VALUE_ORDINAL)) {
return null;
} else {
return invoke(input.getInt(NUM_BUCKETS_ORDINAL), input.getInt(VALUE_ORDINAL));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like a typo. Shall it still be input.getUTF8String()?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Updated!

return null;
}

// TODO - We can probably hash the bytes directly given they're already UTF-8 input.
Copy link
Contributor

@aokolnychyi aokolnychyi Aug 18, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@aokolnychyi aokolnychyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @kbendick! I think there is one typo but looks good otherwise.

@rdblue rdblue merged commit 58368f5 into apache:master Aug 19, 2022
@kbendick kbendick deleted the kb-backport-formatting-changes-to-function-catalog branch August 19, 2022 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants