HIVE-26551: Support CREATE TABLE LIKE FILE for ORC #3611
Conversation
zhangbutao
force-pushed
the
HIVE-26551
branch
from
f4471a0 to
98e50ce
Compare
zhangbutao
force-pushed
the
HIVE-26551
branch
from
98e50ce to
0450999
Compare
kgyrtkirk
added
tests pending
tests passed
and removed
tests unstable
tests pending
labels
|
Thanks for doing this work @zhangbutao ! Very happy to see ORC support. I'll take a look and give this a try on my machine. |
| buffer.append('`'); | ||
| buffer.append(name.replace("`", "``")); | ||
| buffer.append('`'); |
There was a problem hiding this comment.
nit: Concurrent StringBuilder Can be reused.
buffer.append('`').append(name.replace("`", "``")).append('`');
| } | ||
|
|
||
| static void getStructFieldName(StringBuilder buffer, String name) { | ||
| if (UNQUOTED_NAMES.matcher(name).matches()) { |
There was a problem hiding this comment.
Is the intent of this just to ensure there aren't any quoted name, I am not sure, but do explore
!name.startsWith("'") && !name.endsWith("'")
Which lands up being cheaper.
There was a problem hiding this comment.
Actually, this code snippet was borrowed from ORC repo.
https://github.com/apache/orc/blob/6a74eef74b8101c7396f196f1a27f5e124a005f6/java/core/src/java/org/apache/orc/TypeDescription.java#L667-L673
And related ORC jira is https://issues.apache.org/jira/browse/ORC-104. I didn't do much research here, but i think it is better to be sync with ORC code. wdyt?
There was a problem hiding this comment.
should be fine, was just exploring a possibility. :-)
| CTLF_MISSING_STORAGE_FORMAT_DESCRIPTOR(20021, "Failed to find StorageFormatDescriptor for file format ''{0}''", true), | ||
| PARQUET_FOOTER_ERROR(20022, "Failed to read parquet footer:"), | ||
| PARQUET_UNHANDLED_TYPE(20023, "Unhandled type {0}", true), | ||
| ORC_FOOTER_ERROR(20024, "Failed to read orc footer:"), |
There was a problem hiding this comment.
Well the colon is there in PARQUET_FOOTER_ERROR but I am not sure why it is so, when this exception is thrown, post the colon it is the trace, should have been a period according to me, but let it be to be in sync with PARQUET_FOOTER_ERROR. But just out of curiosity if you know the reason behind it do let me know
There was a problem hiding this comment.
I just follow the PARQUET_FOOTER_ERROR, and we can let @jfsii give a explanation. :)
There was a problem hiding this comment.
The source for the change was a comment on a review I had on the patch:
"nit: add a ":" such that the exception is printed after the delimiter"
I could have pushed back on the comment.
zhangbutao
force-pushed
the
HIVE-26551
branch
from
0450999 to
9cd73d0
Compare
|
Kudos, SonarCloud Quality Gate passed!
|
| CTLF_MISSING_STORAGE_FORMAT_DESCRIPTOR(20021, "Failed to find StorageFormatDescriptor for file format ''{0}''", true), | ||
| PARQUET_FOOTER_ERROR(20022, "Failed to read parquet footer:"), | ||
| PARQUET_UNHANDLED_TYPE(20023, "Unhandled type {0}", true), | ||
| ORC_FOOTER_ERROR(20024, "Failed to read orc footer:"), |
There was a problem hiding this comment.
The source for the change was a comment on a review I had on the patch:
"nit: add a ":" such that the exception is printed after the delimiter"
I could have pushed back on the comment.
|
Is this committable? I don't have commit privs and I don't want to see this patch go stale. cc: @ayushtkn |
…angbutao, reviewed by John Sherman, Ayush Saxena)
…angbutao, reviewed by John Sherman, Ayush Saxena)
…angbutao, reviewed by John Sherman, Ayush Saxena)
What changes were proposed in this pull request?
Why are the changes needed?
CREATE TABLE LIKE syntax was added in #3443, and this PR added the ability to create table based on existing orc files.
It is convenient for user to create a table based on a orc file.
Does this PR introduce any user-facing change?
NO
How was this patch tested?
mvn test -Dtest=TestMiniLlapLocalCliDriver -Dqfile=create_table_like_file_orc.q -Dtest.output.overwrite=true