-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Fix missing file extension in Hive connector output files #25787
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
base: master
Are you sure you want to change the base?
Fix missing file extension in Hive connector output files #25787
Conversation
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.
Any testing based on "$path" column ? Does hive also output the data with their extension ?
plugin/trino-delta-lake/src/main/java/io/trino/plugin/deltalake/DeltaLakeMetadata.java
Outdated
Show resolved
Hide resolved
lib/trino-metastore/src/main/java/io/trino/metastore/StorageFormat.java
Outdated
Show resolved
Hide resolved
plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveStorageFormat.java
Show resolved
Hide resolved
@@ -675,7 +675,7 @@ public static int getBucketFromFileName(String fileName) | |||
public static String getFileExtension(HiveCompressionCodec compression, StorageFormat format) | |||
{ | |||
// text format files must have the correct extension when compressed | |||
return compression.getHiveCompressionKind() | |||
return format.getFileExtension() + compression.getHiveCompressionKind() |
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.
move "." here
plugin/trino-hive/src/test/java/io/trino/plugin/hive/metastore/TestStorage.java
Outdated
Show resolved
Hide resolved
c7888b2
to
4b61f65
Compare
@@ -240,7 +240,12 @@ public static Optional<Boolean> directoryExists(TrinoFileSystem fileSystem, Loca | |||
|
|||
public static boolean isFileCreatedByQuery(String fileName, String queryId) | |||
{ | |||
return fileName.startsWith(queryId) || fileName.endsWith(queryId); | |||
return fileName.startsWith(queryId) |
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.
we are using the logic to build fileName/Path, and expect the fileNames always startsWith or endsWith queryId, while the HiveWriteFactory#getFileExtension
possible returns extension, it seems a bug previously
5dcb866
to
a217b4b
Compare
Description
Additional context and related issues
Release notes
(x) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: