-
Notifications
You must be signed in to change notification settings - Fork 29.1k
[SPARK-33172][SQL] Adding support for UserDefinedType for Spark SQL Code generator #30372
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Can one of the admins verify this patch? |
|
In order to verify it first you need to create a table in BigQuery in the following manner: The files are:
A GCP account is needed for that, but the amount of data and operation are well in the free tier. Run Notice that when the format is changed to |
| } else { | ||
| getValue(vector, dataType, rowId) | ||
| dataType match { | ||
| case udt: UserDefinedType[_] => getValueFromVector(vector, udt.sqlType, rowId) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this issue only happens when using spark-bigquery-with-dependencies? In the current spark codebase, it seems dataType cannot be an user-defined type in this method.
| } | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: revert this (unnecessary change)
|
We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable. |
This PR is based on the master branch, replacing PR #30071
What changes were proposed in this pull request?
Having
CodeGenerator.getValueFromVector()to correctly treatUserDefniedTypes asCodeGenerator.javaType()does.Why are the changes needed?
Without it the generated java code would not compile, the error was
The fix makes sure the method call has just one parameter.
Does this PR introduce any user-facing change?
No
How was this patch tested?
I've added a unit test to verify the proper code is generated:
getStruct(ordinal)