-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
PARQUET-1128: [Java] Upgrade the Apache Arrow version to 0.8.0 for SchemaConverter #443
Conversation
…hemaConverter Upgrade Apache Arrow version to 0.8.0. Author: Masayuki Takahashi <[email protected]>
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.
+1, LGTM
case TIMESTAMP_MILLIS: | ||
return field(new ArrowType.Timestamp(org.apache.arrow.vector.types.TimeUnit.MILLISECOND, "UTC")); | ||
case TIME_MILLIS: | ||
return field(new ArrowType.Date(DateUnit.MILLISECOND)); |
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.
Why not Time(MILLISECOND) ?
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.
@julienledem I made a mistake. I will fix it. Thanks.
Thanks for you contribution. see my question above. |
…hemaConverter Fix the wrong conversion from TIME_MILLIS(Parquet) to Time(Arrow). Author: Masayuki Takahashi <[email protected]>
@@ -471,6 +469,7 @@ public TypeMapping convertINT64(PrimitiveTypeName primitiveTypeName) throws Runt | |||
case LIST: | |||
case MAP: | |||
case MAP_KEY_VALUE: | |||
case TIME_MILLIS: | |||
throw new IllegalArgumentException("illegal type " + type); | |||
} |
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.
@julienledem @xhochy And I changed above in SchemaConverter#convertINT64 because TIME_MILLIS only takes int32.
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.
@julienledem @xhochy Could you please check this?
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.
@xhochy I would like this too, please
When I converted parquet(1.9.1-SNAPSHOT) schema to arrow(0.4.0) with SchemaConverter, this exception raised.
This reason is that SchemaConverter refer to Apache Arrow 0.1.0.
I upgrade the Apache Arrow version to 0.8.0(latest) for SchemaConverter.