Skip to content

Commit

Permalink
Review comments fix and Support for Java 21 LTS version
Browse files Browse the repository at this point in the history
  • Loading branch information
AbhijitKulkarni1 committed Jun 17, 2024
1 parent 213e86c commit ee45576
Show file tree
Hide file tree
Showing 13 changed files with 65 additions and 104 deletions.
12 changes: 6 additions & 6 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -2577,9 +2577,9 @@
</build>
</profile>
<profile>
<id>prestoJava19</id>
<id>java21Presto</id>
<activation>
<jdk>19</jdk>
<jdk>21</jdk>
</activation>
<dependencyManagement>
<dependencies>
Expand All @@ -2593,22 +2593,22 @@
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm-tree</artifactId>
<version>9.3</version>
<version>9.5</version>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm-util</artifactId>
<version>9.3</version>
<version>9.5</version>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm-analysis</artifactId>
<version>9.3</version>
<version>9.5</version>
</dependency>
<dependency>
<groupId>org.ow2.asm</groupId>
<artifactId>asm</artifactId>
<version>9.3</version>
<version>9.5</version>
</dependency>
</dependencies>
</dependencyManagement>
Expand Down
8 changes: 4 additions & 4 deletions presto-common/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -118,9 +118,9 @@

<profiles>
<profile>
<id>jdk19PrestoCommon</id>
<id>java21PrestoCommon</id>
<activation>
<jdk>19</jdk>
<jdk>21</jdk>
</activation>
<build>
<plugins>
Expand Down Expand Up @@ -182,9 +182,9 @@
</build>
</profile>
<profile>
<id>lessThanJava19PrestoCommon</id>
<id>lessThanJava21PrestoCommon</id>
<activation>
<jdk>[1.8,19)</jdk>
<jdk>[1.8,21)</jdk>
</activation>
<build>
<plugins>
Expand Down
3 changes: 2 additions & 1 deletion presto-docs/src/main/sphinx/connector/hive.rst
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,8 @@ Property Name Description
``hive.skip-empty-files`` Enable skipping empty files. Otherwise, it will produce an ``false``
error iterating through empty files.

``hive.orc-use-vector-filter`` Enable use of vector ORC readers in compilation and execution ``false``
``hive.orc-use-vector-filter`` Enable use of vector ORC readers in compilation and ``false``
execution
================================================== ============================================================ ============

Metastore Configuration Properties
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -292,12 +292,14 @@ public static ConnectorPageSource createOrcPageSource(
DataSize maxMergeDistance = getOrcMaxMergeDistance(session);
DataSize tinyStripeThreshold = getOrcTinyStripeThreshold(session);
DataSize maxReadBlockSize = getOrcMaxReadBlockSize(session);
boolean orcUseVectorFilter = getOrcUseVectorFilter(session);
OrcReaderOptions orcReaderOptions = OrcReaderOptions.builder()
.withMaxMergeDistance(maxMergeDistance)
.withTinyStripeThreshold(tinyStripeThreshold)
.withMaxBlockSize(maxReadBlockSize)
.withZstdJniDecompressionEnabled(isOrcZstdJniDecompressionEnabled(session))
.withAppendRowNumber(appendRowNumberEnabled || supplyRowIDs)
.withOrcUseVectorFilter(orcUseVectorFilter)
.build();
OrcAggregatedMemoryContext systemMemoryUsage = new HiveOrcAggregatedMemoryContext();
try {
Expand Down Expand Up @@ -366,9 +368,6 @@ public static ConnectorPageSource createOrcPageSource(

List<FilterFunction> filterFunctions = toFilterFunctions(replaceExpression(remainingPredicate, variableToInput), bucketAdapter, session, rowExpressionService.getDeterminismEvaluator(), rowExpressionService.getPredicateCompiler());

boolean orcUseVectorFilter = getOrcUseVectorFilter(session);
Map<String, Boolean> orcReaderUserOptions = new HashMap<String, Boolean>();
orcReaderUserOptions.put(OrcReader.ORC_USE_VECTOR_FILTER, orcUseVectorFilter);
OrcSelectiveRecordReader recordReader = reader.createSelectiveRecordReader(
columnTypes,
outputIndices,
Expand All @@ -384,8 +383,7 @@ public static ConnectorPageSource createOrcPageSource(
hiveStorageTimeZone,
systemMemoryUsage,
Optional.empty(),
INITIAL_BATCH_SIZE,
orcReaderUserOptions);
INITIAL_BATCH_SIZE);

return new OrcSelectivePageSource(
recordReader,
Expand Down
8 changes: 4 additions & 4 deletions presto-orc/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -212,9 +212,9 @@
</build>
</profile>
<profile>
<id>java19PrestoORC</id>
<id>java21PrestoORC</id>
<activation>
<jdk>19</jdk>
<jdk>21</jdk>
</activation>
<dependencies>
<dependency>
Expand Down Expand Up @@ -306,9 +306,9 @@
</build>
</profile>
<profile>
<id>lessThanJdk19PrestoOrc</id>
<id>lessThanJava21PrestoOrc</id>
<activation>
<jdk>[1.8,19)</jdk>
<jdk>[1.8,21)</jdk>
</activation>
<build>
<plugins>
Expand Down
59 changes: 1 addition & 58 deletions presto-orc/src/main/java/com/facebook/presto/orc/OrcReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -370,62 +370,6 @@ public OrcBatchRecordReader createBatchRecordReader(
runtimeStats);
}

public OrcSelectiveRecordReader createSelectiveRecordReader(
Map<Integer, Type> includedColumns,
List<Integer> outputColumns,
Map<Integer, Map<Subfield, TupleDomainFilter>> filters,
List<FilterFunction> filterFunctions,
Map<Integer, Integer> filterFunctionInputs,
Map<Integer, List<Subfield>> requiredSubfields,
Map<Integer, Object> constantValues,
Map<Integer, Function<Block, Block>> coercers,
OrcPredicate predicate,
long offset,
long length,
DateTimeZone hiveStorageTimeZone,
OrcAggregatedMemoryContext systemMemoryUsage,
Optional<OrcWriteValidation> writeValidation,
int initialBatchSize,
Map<String, Boolean> orcReaderUserOptions)
{
return new OrcSelectiveRecordReader(
includedColumns,
outputColumns,
filters,
filterFunctions,
filterFunctionInputs,
requiredSubfields,
constantValues,
coercers,
predicate,
footer.getNumberOfRows(),
footer.getStripes(),
footer.getFileStats(),
metadata.getStripeStatsList(),
getOrcDataSource(),
offset,
length,
footer.getTypes(),
decompressor,
encryptionLibrary,
dwrfEncryptionGroupMap,
columnsToIntermediateKeys,
footer.getRowsInRowGroup(),
hiveStorageTimeZone,
new OrcRecordReaderOptions(orcReaderOptions),
hiveWriterVersion,
metadataReader,
footer.getUserMetadata(),
systemMemoryUsage.newOrcAggregatedMemoryContext(),
writeValidation,
initialBatchSize,
stripeMetadataSource,
cacheable,
runtimeStats,
fileIntrospector,
orcReaderUserOptions);
}

public OrcSelectiveRecordReader createSelectiveRecordReader(
Map<Integer, Type> includedColumns,
List<Integer> outputColumns,
Expand Down Expand Up @@ -477,8 +421,7 @@ public OrcSelectiveRecordReader createSelectiveRecordReader(
stripeMetadataSource,
cacheable,
runtimeStats,
fileIntrospector,
null);
fileIntrospector);
}

private static OrcDataSource wrapWithCacheIfTiny(OrcDataSource dataSource, DataSize maxCacheSize, OrcAggregatedMemoryContext systemMemoryContext)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ public class OrcReaderOptions
private final boolean mapNullKeysEnabled;
// if the option is set to true, OrcSelectiveReader will append a row number block at the end of the page
private final boolean appendRowNumber;
// if the option is set to true ORCSelectiveReader will use vector filter logic if supported
private final boolean orcUseVectorFilter;

/**
* Read column statistics for flat map columns. Usually there are quite a
Expand All @@ -41,7 +43,8 @@ private OrcReaderOptions(
boolean zstdJniDecompressionEnabled,
boolean mapNullKeysEnabled,
boolean appendRowNumber,
boolean readMapStatistics)
boolean readMapStatistics,
boolean orcUseVectorFilter)
{
this.maxMergeDistance = requireNonNull(maxMergeDistance, "maxMergeDistance is null");
this.maxBlockSize = requireNonNull(maxBlockSize, "maxBlockSize is null");
Expand All @@ -50,6 +53,7 @@ private OrcReaderOptions(
this.mapNullKeysEnabled = mapNullKeysEnabled;
this.appendRowNumber = appendRowNumber;
this.readMapStatistics = readMapStatistics;
this.orcUseVectorFilter = orcUseVectorFilter;
}

public DataSize getMaxMergeDistance()
Expand Down Expand Up @@ -82,6 +86,11 @@ public boolean appendRowNumber()
return appendRowNumber;
}

public boolean getOrcUseVectorFilter()
{
return orcUseVectorFilter;
}

public boolean readMapStatistics()
{
return readMapStatistics;
Expand All @@ -98,6 +107,7 @@ public String toString()
.add("mapNullKeysEnabled", mapNullKeysEnabled)
.add("appendRowNumber", appendRowNumber)
.add("readMapStatistics", readMapStatistics)
.add("orcUseVectorFilter", orcUseVectorFilter)
.toString();
}

Expand All @@ -114,6 +124,7 @@ public static final class Builder
private boolean zstdJniDecompressionEnabled;
private boolean mapNullKeysEnabled;
private boolean appendRowNumber;
private boolean orcUseVectorFilter;
private boolean readMapStatistics;

private Builder() {}
Expand Down Expand Up @@ -154,6 +165,12 @@ public Builder withAppendRowNumber(boolean appendRowNumber)
return this;
}

public Builder withOrcUseVectorFilter(boolean orcUseVectorFilter)
{
this.orcUseVectorFilter = orcUseVectorFilter;
return this;
}

public Builder withReadMapStatistics(boolean readMapStatistics)
{
this.readMapStatistics = readMapStatistics;
Expand All @@ -169,7 +186,8 @@ public OrcReaderOptions build()
zstdJniDecompressionEnabled,
mapNullKeysEnabled,
appendRowNumber,
readMapStatistics);
readMapStatistics,
orcUseVectorFilter);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -24,24 +24,28 @@ public class OrcRecordReaderOptions
private final DataSize maxBlockSize;
private final boolean mapNullKeysEnabled;
private final boolean appendRowNumber;
private final boolean orcUseVectorFilter;

public OrcRecordReaderOptions(OrcReaderOptions options)
{
this(options.getMaxMergeDistance(), options.getTinyStripeThreshold(), options.getMaxBlockSize(), options.mapNullKeysEnabled(), options.appendRowNumber());
this(options.getMaxMergeDistance(), options.getTinyStripeThreshold(), options.getMaxBlockSize(),
options.mapNullKeysEnabled(), options.appendRowNumber(), options.getOrcUseVectorFilter());
}

public OrcRecordReaderOptions(
DataSize maxMergeDistance,
DataSize tinyStripeThreshold,
DataSize maxBlockSize,
boolean mapNullKeysEnabled,
boolean appendRowNumber)
boolean appendRowNumber,
boolean orcUseVectorFilter)
{
this.maxMergeDistance = requireNonNull(maxMergeDistance, "maxMergeDistance is null");
this.maxBlockSize = requireNonNull(maxBlockSize, "maxBlockSize is null");
this.tinyStripeThreshold = requireNonNull(tinyStripeThreshold, "tinyStripeThreshold is null");
this.mapNullKeysEnabled = mapNullKeysEnabled;
this.appendRowNumber = appendRowNumber;
this.orcUseVectorFilter = orcUseVectorFilter;
}

public DataSize getMaxMergeDistance()
Expand All @@ -68,4 +72,9 @@ public boolean appendRowNumber()
{
return appendRowNumber;
}

public boolean getOrcUseVectorFilter()
{
return orcUseVectorFilter;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -188,8 +188,7 @@ public OrcSelectiveRecordReader(
StripeMetadataSource stripeMetadataSource,
boolean cacheable,
RuntimeStats runtimeStats,
Optional<OrcFileIntrospector> fileIntrospector,
Map<String, Boolean> orcReaderUserOptions)
Optional<OrcFileIntrospector> fileIntrospector)
{
super(includedColumns,
requiredSubfields,
Expand All @@ -204,8 +203,7 @@ public OrcSelectiveRecordReader(
filterFunctions,
filterFunctionInputMapping,
requiredSubfields,
systemMemoryUsage.newOrcAggregatedMemoryContext(),
orcReaderUserOptions),
systemMemoryUsage.newOrcAggregatedMemoryContext()),
predicate,
numberOfRows,
fileStripes,
Expand Down Expand Up @@ -591,8 +589,7 @@ private static SelectiveStreamReader[] createStreamReaders(
List<FilterFunction> filterFunctions,
Map<Integer, Integer> filterFunctionInputMapping,
Map<Integer, List<Subfield>> requiredSubfields,
OrcAggregatedMemoryContext systemMemoryContext,
Map<String, Boolean> orcReaderUserOptions)
OrcAggregatedMemoryContext systemMemoryContext)
{
List<StreamDescriptor> streamDescriptors = createStreamDescriptor(types, orcDataSource).getNestedStreams();

Expand All @@ -619,8 +616,7 @@ private static SelectiveStreamReader[] createStreamReaders(
hiveStorageTimeZone,
options,
systemMemoryContext,
false,
orcReaderUserOptions);
false);
}
}
return streamReaders;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,8 +147,8 @@ public MapDirectSelectiveStreamReader(
.collect(toImmutableList());
}

this.keyReader = SelectiveStreamReaders.createStreamReader(nestedStreams.get(0), keyFilter, keyOutputType, ImmutableList.of(), hiveStorageTimeZone, options, systemMemoryContext.newOrcAggregatedMemoryContext(), isLowMemory, null);
this.valueReader = SelectiveStreamReaders.createStreamReader(nestedStreams.get(1), ImmutableMap.of(), valueOutputType, elementRequiredSubfields, hiveStorageTimeZone, options, systemMemoryContext.newOrcAggregatedMemoryContext(), isLowMemory, null);
this.keyReader = SelectiveStreamReaders.createStreamReader(nestedStreams.get(0), keyFilter, keyOutputType, ImmutableList.of(), hiveStorageTimeZone, options, systemMemoryContext.newOrcAggregatedMemoryContext(), isLowMemory);
this.valueReader = SelectiveStreamReaders.createStreamReader(nestedStreams.get(1), ImmutableMap.of(), valueOutputType, elementRequiredSubfields, hiveStorageTimeZone, options, systemMemoryContext.newOrcAggregatedMemoryContext(), isLowMemory);
}
else {
this.keyReader = null;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -679,8 +679,7 @@ public void startStripe(Stripe stripe)
hiveStorageTimeZone,
options,
systemMemoryContext.newOrcAggregatedMemoryContext(),
true,
null);
true);
valueStreamReader.startStripe(stripe);
valueStreamReaders.add(valueStreamReader);
}
Expand Down
Loading

0 comments on commit ee45576

Please sign in to comment.