Skip to content

Commit e486b09

Browse files
committed
Remove HiveConfig Feature Flag
The native Ion Feature Flag was not being used in Athena and was causing test failures after the rebase. Since OSS Trino never had Ion support there is no reason to keep it.
1 parent 85cac6d commit e486b09

File tree

7 files changed

+12
-77
lines changed

7 files changed

+12
-77
lines changed

Diff for: plugin/trino-hive/src/main/java/io/trino/plugin/hive/HiveConfig.java

-19
Original file line numberDiff line numberDiff line change
@@ -111,12 +111,6 @@ public class HiveConfig
111111

112112
private String orcLegacyTimeZone = TimeZone.getDefault().getID();
113113

114-
// This is a feature flag for Ion native trino integration.
115-
// Default value is false and requires explicitly setting it to true to enable Ion native trino integration.
116-
// TODO: This should change to default as true in future once we have a complete implementation of Ion native
117-
// trino integration supported.
118-
private boolean ionNativeTrinoEnabled;
119-
120114
private String parquetTimeZone = TimeZone.getDefault().getID();
121115
private boolean useParquetColumnNames = true;
122116

@@ -720,19 +714,6 @@ public HiveConfig setOrcLegacyTimeZone(String orcLegacyTimeZone)
720714
return this;
721715
}
722716

723-
public boolean getIonNativeTrinoEnabled()
724-
{
725-
return ionNativeTrinoEnabled;
726-
}
727-
728-
@Config("hive.ion.nativetrino")
729-
@ConfigDescription("Feature flag to enable Ion native trino integration")
730-
public HiveConfig setIonNativeTrinoEnabled(boolean ionNativeTrinoEnabled)
731-
{
732-
this.ionNativeTrinoEnabled = ionNativeTrinoEnabled;
733-
return this;
734-
}
735-
736717
public DateTimeZone getParquetDateTimeZone()
737718
{
738719
TimeZone timeZone = TimeZone.getTimeZone(ZoneId.of(parquetTimeZone));

Diff for: plugin/trino-hive/src/main/java/io/trino/plugin/hive/ion/IonFileWriterFactory.java

+2-7
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@
2323
import io.trino.metastore.StorageFormat;
2424
import io.trino.plugin.hive.FileWriter;
2525
import io.trino.plugin.hive.HiveCompressionCodec;
26-
import io.trino.plugin.hive.HiveConfig;
2726
import io.trino.plugin.hive.HiveFileWriterFactory;
2827
import io.trino.plugin.hive.WriterKind;
2928
import io.trino.plugin.hive.acid.AcidTransaction;
@@ -52,17 +51,14 @@ public class IonFileWriterFactory
5251
{
5352
private final TrinoFileSystemFactory fileSystemFactory;
5453
private final TypeManager typeManager;
55-
private final boolean nativeTrinoEnabled;
5654

5755
@Inject
5856
public IonFileWriterFactory(
5957
TrinoFileSystemFactory fileSystemFactory,
60-
TypeManager typeManager,
61-
HiveConfig hiveConfig)
58+
TypeManager typeManager)
6259
{
6360
this.fileSystemFactory = fileSystemFactory;
6461
this.typeManager = typeManager;
65-
this.nativeTrinoEnabled = hiveConfig.getIonNativeTrinoEnabled();
6662
}
6763

6864
@Override
@@ -78,8 +74,7 @@ public Optional<FileWriter> createFileWriter(
7874
boolean useAcidSchema,
7975
WriterKind writerKind)
8076
{
81-
if (!nativeTrinoEnabled
82-
|| !ION_OUTPUT_FORMAT.equals(storageFormat.getOutputFormat())) {
77+
if (!ION_OUTPUT_FORMAT.equals(storageFormat.getOutputFormat())) {
8378
return Optional.empty();
8479
}
8580

Diff for: plugin/trino-hive/src/main/java/io/trino/plugin/hive/ion/IonPageSourceFactory.java

+2-6
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@
2929
import io.trino.hive.formats.line.Column;
3030
import io.trino.plugin.hive.AcidInfo;
3131
import io.trino.plugin.hive.HiveColumnHandle;
32-
import io.trino.plugin.hive.HiveConfig;
3332
import io.trino.plugin.hive.HivePageSourceFactory;
3433
import io.trino.plugin.hive.Schema;
3534
import io.trino.plugin.hive.acid.AcidTransaction;
@@ -57,13 +56,11 @@ public class IonPageSourceFactory
5756
implements HivePageSourceFactory
5857
{
5958
private final TrinoFileSystemFactory trinoFileSystemFactory;
60-
private final boolean nativeTrinoEnabled;
6159

6260
@Inject
63-
public IonPageSourceFactory(TrinoFileSystemFactory trinoFileSystemFactory, HiveConfig hiveConfig)
61+
public IonPageSourceFactory(TrinoFileSystemFactory trinoFileSystemFactory)
6462
{
6563
this.trinoFileSystemFactory = trinoFileSystemFactory;
66-
this.nativeTrinoEnabled = hiveConfig.getIonNativeTrinoEnabled();
6764
}
6865

6966
@Override
@@ -82,8 +79,7 @@ public Optional<ConnectorPageSource> createPageSource(
8279
boolean originalFile,
8380
AcidTransaction transaction)
8481
{
85-
if (!nativeTrinoEnabled
86-
|| !ION_SERDE_CLASS.equals(schema.serializationLibraryName())) {
82+
if (!ION_SERDE_CLASS.equals(schema.serializationLibraryName())) {
8783
return Optional.empty();
8884
}
8985

Diff for: plugin/trino-hive/src/test/java/io/trino/plugin/hive/HiveTestUtils.java

+2-5
Original file line numberDiff line numberDiff line change
@@ -181,9 +181,6 @@ public static Set<HivePageSourceFactory> getDefaultHivePageSourceFactories(HdfsE
181181
public static Set<HivePageSourceFactory> getDefaultHivePageSourceFactories(TrinoFileSystemFactory fileSystemFactory, HiveConfig hiveConfig)
182182
{
183183
FileFormatDataSourceStats stats = new FileFormatDataSourceStats();
184-
// set IonNativeTrino as true in hiveConfig for testing
185-
// TODO: In future this flag should change to `true` as default and then the following statement can be removed.
186-
hiveConfig.setIonNativeTrinoEnabled(true);
187184
return ImmutableSet.<HivePageSourceFactory>builder()
188185
.add(new CsvPageSourceFactory(fileSystemFactory, hiveConfig))
189186
.add(new JsonPageSourceFactory(fileSystemFactory, hiveConfig))
@@ -195,7 +192,7 @@ public static Set<HivePageSourceFactory> getDefaultHivePageSourceFactories(Trino
195192
.add(new RcFilePageSourceFactory(fileSystemFactory, hiveConfig))
196193
.add(new OrcPageSourceFactory(new OrcReaderConfig(), fileSystemFactory, stats, hiveConfig))
197194
.add(new ParquetPageSourceFactory(fileSystemFactory, stats, new ParquetReaderConfig(), hiveConfig))
198-
.add(new IonPageSourceFactory(fileSystemFactory, hiveConfig))
195+
.add(new IonPageSourceFactory(fileSystemFactory))
199196
.build();
200197
}
201198

@@ -218,7 +215,7 @@ public static Set<HiveFileWriterFactory> getDefaultHiveFileWriterFactories(HiveC
218215
.add(new RcFileFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER, nodeVersion, hiveConfig))
219216
.add(new OrcFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER, nodeVersion, new FileFormatDataSourceStats(), new OrcWriterConfig()))
220217
.add(new ParquetFileWriterFactory(fileSystemFactory, nodeVersion, TESTING_TYPE_MANAGER, hiveConfig, new FileFormatDataSourceStats()))
221-
.add(new IonFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER, hiveConfig))
218+
.add(new IonFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER))
222219
.build();
223220
}
224221

Diff for: plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveConfig.java

-3
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,6 @@ public void testDefaults()
7878
.setParallelPartitionedBucketedWrites(true)
7979
.setTextMaxLineLength(DataSize.of(100, Unit.MEGABYTE))
8080
.setOrcLegacyTimeZone(TimeZone.getDefault().getID())
81-
.setIonNativeTrinoEnabled(false)
8281
.setParquetTimeZone(TimeZone.getDefault().getID())
8382
.setUseParquetColumnNames(true)
8483
.setRcfileTimeZone(TimeZone.getDefault().getID())
@@ -163,7 +162,6 @@ public void testExplicitPropertyMappings()
163162
.put("hive.max-partition-drops-per-query", "1000")
164163
.put("hive.text.max-line-length", "13MB")
165164
.put("hive.orc.time-zone", nonDefaultTimeZone().getID())
166-
.put("hive.ion.nativetrino", "true")
167165
.put("hive.parquet.time-zone", nonDefaultTimeZone().getID())
168166
.put("hive.parquet.use-column-names", "false")
169167
.put("hive.rcfile.time-zone", nonDefaultTimeZone().getID())
@@ -247,7 +245,6 @@ public void testExplicitPropertyMappings()
247245
.setParallelPartitionedBucketedWrites(false)
248246
.setTextMaxLineLength(DataSize.of(13, Unit.MEGABYTE))
249247
.setOrcLegacyTimeZone(nonDefaultTimeZone().getID())
250-
.setIonNativeTrinoEnabled(true)
251248
.setParquetTimeZone(nonDefaultTimeZone().getID())
252249
.setUseParquetColumnNames(false)
253250
.setRcfileTimeZone(nonDefaultTimeZone().getID())

Diff for: plugin/trino-hive/src/test/java/io/trino/plugin/hive/TestHiveFileFormats.java

+2-7
Original file line numberDiff line numberDiff line change
@@ -378,17 +378,12 @@ public void testIonWithBinaryEncoding(int rowCount, long fileSizePadding)
378378
.filter(tc -> !(tc.type instanceof MapType))
379379
.collect(toList());
380380

381-
HiveConfig hiveConfig = new HiveConfig();
382-
// enable Ion native trino integration for testing while the implementation is in progress
383-
// TODO: In future this flag should change to `true` as default and then the following statement can be removed.
384-
hiveConfig.setIonNativeTrinoEnabled(true);
385-
386381
assertThatFileFormat(ION)
387382
.withColumns(testColumns)
388383
.withRowsCount(rowCount)
389384
.withFileSizePadding(fileSizePadding)
390-
.withFileWriterFactory(fileSystemFactory -> new IonFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER, hiveConfig))
391-
.isReadableByPageSource(fileSystemFactory -> new IonPageSourceFactory(fileSystemFactory, hiveConfig));
385+
.withFileWriterFactory(fileSystemFactory -> new IonFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER))
386+
.isReadableByPageSource(fileSystemFactory -> new IonPageSourceFactory(fileSystemFactory));
392387
}
393388

394389
@Test(dataProvider = "validRowAndFileSizePadding")

Diff for: plugin/trino-hive/src/test/java/io/trino/plugin/hive/ion/IonPageSourceSmokeTest.java

+4-30
Original file line numberDiff line numberDiff line change
@@ -141,7 +141,7 @@ public void testStrictAndLaxPathTyping()
141141
TestFixture strictFixture = new TestFixture(FOO_BAR_COLUMNS);
142142
strictFixture.withSerdeProperty(STRICT_PATH_TYPING_PROPERTY, "true");
143143

144-
Assertions.assertThrows(TrinoException.class, () ->
144+
Assertions.assertThrows(RuntimeException.class, () ->
145145
strictFixture.assertRowCount("37 null.timestamp []", 3));
146146
}
147147

@@ -218,23 +218,6 @@ public void testPageSourceTelemetryCompressed()
218218
Assertions.assertTrue(pageSource.getReadTimeNanos() > 0);
219219
}
220220

221-
@Test
222-
public void testNativeTrinoDisabled()
223-
throws IOException
224-
{
225-
TestFixture fixture = new TestFixture(FOO_BAR_COLUMNS)
226-
.withNativeIonDisabled();
227-
228-
Assertions.assertTrue(
229-
fixture.getOptionalFileWriter().isEmpty(),
230-
"Expected empty file writer when native trino is disabled");
231-
232-
fixture.writeIonTextFile("");
233-
Assertions.assertTrue(
234-
fixture.getOptionalPageSource().isEmpty(),
235-
"Expected empty page source when native trino is disabled");
236-
}
237-
238221
private static Stream<Map.Entry<String, String>> propertiesWithDefaults()
239222
{
240223
return Stream.of(
@@ -367,7 +350,6 @@ private static class TestFixture
367350
{
368351
private TrinoFileSystemFactory fileSystemFactory = new MemoryFileSystemFactory();
369352
private Location fileLocation = Location.of(TEST_ION_LOCATION);
370-
private HiveConfig hiveConfig = new HiveConfig();
371353
private Map<String, String> tableProperties = new HashMap<>();
372354
private List<HiveColumnHandle> columns;
373355
private List<HiveColumnHandle> projections;
@@ -389,14 +371,6 @@ private static class TestFixture
389371
tableProperties.put(LIST_COLUMN_TYPES, columns.stream().map(HiveColumnHandle::getHiveType)
390372
.map(HiveType::toString)
391373
.collect(Collectors.joining(",")));
392-
// the default at runtime is false, but most of our testing obviously assumes it is enabled.
393-
hiveConfig.setIonNativeTrinoEnabled(true);
394-
}
395-
396-
TestFixture withNativeIonDisabled()
397-
{
398-
hiveConfig.setIonNativeTrinoEnabled(false);
399-
return this;
400374
}
401375

402376
TestFixture withSerdeProperty(String key, String value)
@@ -408,7 +382,7 @@ TestFixture withSerdeProperty(String key, String value)
408382
Optional<ConnectorPageSource> getOptionalPageSource()
409383
throws IOException
410384
{
411-
IonPageSourceFactory pageSourceFactory = new IonPageSourceFactory(fileSystemFactory, hiveConfig);
385+
IonPageSourceFactory pageSourceFactory = new IonPageSourceFactory(fileSystemFactory);
412386

413387
long length = fileSystemFactory.create(getSession()).newInputFile(fileLocation).length();
414388
long nowMillis = Instant.now().toEpochMilli();
@@ -453,7 +427,7 @@ ConnectorPageSource getPageSource()
453427
ConnectorSession getSession()
454428
{
455429
if (session == null) {
456-
session = getHiveSession(hiveConfig);
430+
session = getHiveSession(new HiveConfig());
457431
}
458432
return session;
459433
}
@@ -483,7 +457,7 @@ int writeZstdCompressedIonText(String ionText)
483457

484458
Optional<FileWriter> getOptionalFileWriter()
485459
{
486-
return new IonFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER, hiveConfig)
460+
return new IonFileWriterFactory(fileSystemFactory, TESTING_TYPE_MANAGER)
487461
.createFileWriter(
488462
fileLocation,
489463
columns.stream().map(HiveColumnHandle::getName).collect(toList()),

0 commit comments

Comments
 (0)