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
Build AcidInfo only for ORC-ACID tables #8259
Build AcidInfo only for ORC-ACID tables #8259
Conversation
AcidInfo.Builder acidInfoBuilder, | ||
boolean isFullAcid) |
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.
should acidInfoBuilder
be Optional?
" 'line.delim'='\\n',\n" + | ||
" 'serialization.format'=',')\n" + | ||
" STORED AS INPUTFORMAT\n" + | ||
" 'org.apache.hadoop.mapred.TextInputFormat'\n" + |
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 should cover Parquet as well.
Can there be some reuse between testReadInsertOnly
? ideally they different in table file format definition?
@@ -238,6 +238,63 @@ public void testReadInsertOnly(boolean isPartitioned, BucketingType bucketingTyp | |||
} | |||
} | |||
|
|||
@Test(groups = HIVE_TRANSACTIONAL, dataProvider = "partitioningAndBucketingTypeDataProvider", timeOut = TEST_TIMEOUT) | |||
@Flaky(issue = "https://github.com/trinodb/trino/issues/4927", match = "Hive table .* is corrupt. Found sub-directory in bucket directory for partition") | |||
public void testReadNonOrcInsertOnly(boolean isPartitioned, BucketingType bucketingType) |
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.
bucketingType.getHiveClustering(
seems to be missing, so bucketingType is effectively ignored.
@@ -238,6 +238,63 @@ public void testReadInsertOnly(boolean isPartitioned, BucketingType bucketingTyp | |||
} | |||
} | |||
|
|||
@Test(groups = HIVE_TRANSACTIONAL, dataProvider = "partitioningAndBucketingTypeDataProvider", timeOut = TEST_TIMEOUT) |
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.
Maybe we don't need all 4 combinations.
fileIterators.addLast(createInternalHiveSplitIterator(readPath, fs, splitFactory, splittable, acidInfoBuilder.build())); | ||
Optional<AcidInfo> acidInfo = Optional.empty(); | ||
if (isFullAcid) { | ||
acidInfo = acidInfoBuilder.build(); |
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.
since the method is long anyway, let's use ?:
for brevity and to have variables final.
Merged 79b34c5, thanks! |
No description provided.