Skip to content

Spark: Fix ClassCastException when using bucket UDF - #3368

Merged
rdblue merged 7 commits into
apache:masterfrom
izchen:fix_bucket_udf
Oct 26, 2021
Merged

Spark: Fix ClassCastException when using bucket UDF#3368
rdblue merged 7 commits into
apache:masterfrom
izchen:fix_bucket_udf

Conversation

@izchen

@izchenizchen commented Oct 25, 2021

Copy link
Copy Markdown
Contributor

Currently, directly register iceberg api org.apache.iceberg.transforms.Bucket#apply as spark UDF in org.apache.iceberg.spark#IcebergSpark.

For byte, short, date, timestamp, and binary, Spark value of these types is different from iceberg‘s internal representation, which will cause a ClassCastException error.

SPARK TYPESPARK VALUEICEBERG TYPEICEBERG VALUE
ByteTypejava.lang.ByteIntegerTypejava.lang.Integer
ShortTypejava.lang.ShortIntegerTypejava.lang.Integer
DateTypejava.sql.DateDateTypejava.lang.Integer
TimestampTypejava.sql.TimestampTimestampType.withZonejava.lang.Long
BinaryTypebyte arrayBinaryTypejava.nio.ByteBuffer

We should first convert the spark value to iceberg's internal representation, and then use the converted value as the input of iceberg api org.apache.iceberg.spark#IcebergSpark.

In addition, add more ut in this PR to cover all spark atom types.

@izchen

Copy link
Copy Markdown
ContributorAuthor

Related issue: #2838

@izchen

Copy link
Copy Markdown
ContributorAuthor

@rdblue@RussellSpitzer , could you help to review this PR? :)

}

@SuppressWarnings("checkstyle:CyclomaticComplexity")
public static Object convertAtomicValue(DataType atomic, Object object) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is this needed?

I think you could handle short and byte types by updating the convert(Type, Object) method above:

caseINTEGER:
return ((Number) object).intValue();

Then you wouldn't need a new method at all.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Thanks for your review. The method is not needed.

@rdblue

Copy link
Copy Markdown
Contributor

@izchen, could you fix just the latest version of Spark and then we'll port the changes to the other versions when after it is merged? That way we don't have a commit that affects all Spark versions.

zhangchen added 3 commits October 26, 2021 00:41
@izchen

Copy link
Copy Markdown
ContributorAuthor

@rdblue done, could you help to review this PR again?

@jackye1995jackye1995 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

looks good to me

@rdbluerdblue added this to the Java 0.12.1 Release milestone Oct 26, 2021
@rdblue

Copy link
Copy Markdown
Contributor

Looks good. Thanks, @izchen!

@rdblue
rdblue merged commit 425641a into apache:masterOct 26, 2021
@izchen
izchen deleted the fix_bucket_udf branch October 27, 2021 01:45
@izchen

Copy link
Copy Markdown
ContributorAuthor

Thanks, @rdblue@jackye1995 !

kbendick pushed a commit to kbendick/iceberg that referenced this pull request Oct 27, 2021
kbendick pushed a commit to kbendick/iceberg that referenced this pull request Nov 1, 2021
wypoon added a commit to wypoon/iceberg that referenced this pull request Nov 18, 2021
wypoon added a commit to wypoon/iceberg that referenced this pull request Nov 18, 2021
wypoon added a commit to wypoon/iceberg that referenced this pull request Nov 18, 2021
rdblue pushed a commit that referenced this pull request Nov 18, 2021
rdblue pushed a commit that referenced this pull request Nov 18, 2021
rdblue pushed a commit that referenced this pull request Nov 18, 2021
Initial-neko pushed a commit to Initial-neko/iceberg that referenced this pull request Nov 23, 2021
Initial-neko pushed a commit to Initial-neko/iceberg that referenced this pull request Nov 23, 2021
Initial-neko pushed a commit to Initial-neko/iceberg that referenced this pull request Nov 23, 2021
izchen added a commit to izchen/iceberg that referenced this pull request Dec 7, 2021
Sign up for freeto 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.

3 participants

@izchen@rdblue@jackye1995