abstractdog commented on code in PR #5648:
URL: https://github.com/apache/hive/pull/5648#discussion_r1977173425
##########
iceberg/iceberg-handler/src/test/java/org/apache/iceberg/mr/hive/HiveIcebergStorageHandlerWithEngineBase.java:
##########
@@ -108,57 +108,46 @@ public abstract class
HiveIcebergStorageHandlerWithEngineBase {
StatsSetupConst.TOTAL_SIZE, SnapshotSummary.TOTAL_FILE_SIZE_PROP
);
- @Parameters(name = "fileFormat={0}, engine={1}, catalog={2},
isVectorized={3}, formatVersion={4}")
+ @Parameters(name = "fileFormat={0}, catalog={1}, isVectorized={2},
formatVersion={3}")
public static Collection<Object[]> parameters() {
Collection<Object[]> testParams = Lists.newArrayList();
- String javaVersion = System.getProperty("java.specification.version");
// Run tests with every FileFormat for a single Catalog (HiveCatalog)
for (FileFormat fileFormat :
HiveIcebergStorageHandlerTestUtils.FILE_FORMATS) {
- for (String engine : EXECUTION_ENGINES) {
- IntStream.of(2, 1).forEach(formatVersion -> {
- // include Tez tests only for Java 8
- if (javaVersion.equals("1.8")) {
- testParams.add(new Object[]{fileFormat, engine,
TestTables.TestTableType.HIVE_CATALOG, false,
- formatVersion});
- // test for vectorization=ON in case of ORC and PARQUET format
with Tez engine
- if (fileFormat != FileFormat.METADATA && "tez".equals(engine) &&
HiveVersion.min(HiveVersion.HIVE_3)) {
- testParams.add(new Object[]{fileFormat, engine,
TestTables.TestTableType.HIVE_CATALOG, true,
- formatVersion});
- }
- }
- });
- }
+ IntStream.of(2, 1).forEach(formatVersion -> {
+ testParams.add(new Object[]{fileFormat, HIVE_CATALOG, false,
formatVersion});
+ // test for vectorization=ON in case of ORC and PARQUET format with
Tez engine
+ if (fileFormat != FileFormat.METADATA) {
+ testParams.add(new Object[]{fileFormat, HIVE_CATALOG, true,
formatVersion});
+ }
+ });
}
- // Run tests for every Catalog for a single FileFormat (PARQUET) and
execution engine (tez)
- // skip HiveCatalog tests as they are added before
- for (TestTables.TestTableType testTableType : TestTables.ALL_TABLE_TYPES) {
- if (!TestTables.TestTableType.HIVE_CATALOG.equals(testTableType)) {
- testParams.add(new Object[]{FileFormat.PARQUET, "tez", testTableType,
false, 1});
+ // Run tests for every Catalog for a single FileFormat (PARQUET), skip
HiveCatalog tests as they are added before
+ for (TestTableType testTableType : ALL_TABLE_TYPES) {
+ if (testTableType != HIVE_CATALOG) {
+ testParams.add(new Object[]{FileFormat.PARQUET, testTableType, false,
1});
}
}
return testParams;
}
protected static TestHiveShell shell;
+ protected static String executionEngine = "tez";
Review Comment:
hm, this looks strange
a) is it needed at all?
b) if needed, it looks a like a constant, so it's rather:
```
protected static final String EXECUTION_ENGINE = "tez";
```
or something like that
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: gitbox-unsubscr...@hive.apache.org
For additional commands, e-mail: gitbox-h...@hive.apache.org
