bvaradar commented on a change in pull request #1944:
URL: https://github.com/apache/hudi/pull/1944#discussion_r468702061
##########
File path: packaging/hudi-presto-bundle/pom.xml
##########
@@ -76,7 +76,12 @@
<include>org.apache.hbase:hbase-common</include>
<include>org.apache.hbase:hbase-protocol</include>
<include>org.apache.hbase:hbase-server</include>
+ <include>org.apache.hbase:hbase-annotations</include>
<include>org.apache.htrace:htrace-core</include>
+ <include>com.yammer.metrics:metrics-core</include>
+ <include>com.google.guava:guava</include>
Review comment:
@umehrot2 Are these jars not provided by Presto runtime itself ?
Version mismatch with guava had been a regular issue earlier but I see that you
have shaded them.
Please also check the licensing of the new dependencies. Are they Apache ?
##########
File path:
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/utils/HoodieHiveUtils.java
##########
@@ -38,6 +40,9 @@
public static final String HOODIE_CONSUME_MODE_PATTERN =
"hoodie.%s.consume.mode";
public static final String HOODIE_START_COMMIT_PATTERN =
"hoodie.%s.consume.start.timestamp";
public static final String HOODIE_MAX_COMMIT_PATTERN =
"hoodie.%s.consume.max.commits";
+ public static final Set<String> VIRTUAL_COLUMN_NAMES =
CollectionUtils.createImmutableSet(
Review comment:
Is this specific to Parquet or Hive in general. If this is specific to
Parquet, can you move it to ParquetUtils ?
##########
File path: packaging/hudi-presto-bundle/pom.xml
##########
@@ -76,7 +76,12 @@
<include>org.apache.hbase:hbase-common</include>
<include>org.apache.hbase:hbase-protocol</include>
<include>org.apache.hbase:hbase-server</include>
+ <include>org.apache.hbase:hbase-annotations</include>
<include>org.apache.htrace:htrace-core</include>
+ <include>com.yammer.metrics:metrics-core</include>
+ <include>com.google.guava:guava</include>
+ <include>commons-lang:commons-lang</include>
Review comment:
Can you also shade commons-lang and protobuf ?
##########
File path:
hudi-hadoop-mr/src/main/java/org/apache/hudi/hadoop/HoodieParquetInputFormat.java
##########
@@ -63,6 +62,7 @@
* that does not correspond to a hoodie table then they are passed in as is
(as what FileInputFormat.listStatus()
* would do). The JobConf could have paths from multipe Hoodie/Non-Hoodie
tables
*/
+@UseRecordReaderFromInputFormat
Review comment:
Is this the main change ?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]