yihua commented on code in PR #8590:
URL: https://github.com/apache/hudi/pull/8590#discussion_r1179858534
##########
hudi-client/hudi-client-common/src/main/java/org/apache/hudi/config/HoodieWriteConfig.java:
##########
@@ -698,6 +698,7 @@ public class HoodieWriteConfig extends HoodieConfig {
+ "will not print any configuration which contains the configured
filter. For example with "
+ "a configured filter `ssl`, value for config
`ssl.trustore.location` would be masked.");
+
Review Comment:
nit: no need to add new line
##########
hudi-common/src/main/java/org/apache/hudi/avro/HoodieAvroWriteSupport.java:
##########
@@ -35,17 +36,19 @@
/**
* Wrap AvroWriterSupport for plugging in the bloom filter.
*/
-public class HoodieAvroWriteSupport extends AvroWriteSupport {
+public class HoodieAvroWriteSupport<T> extends AvroWriteSupport<T> {
Review Comment:
Seems like this type parameter `<T>` is not needed?
##########
hudi-common/src/main/java/org/apache/hudi/io/storage/HoodieAvroFileWriterFactory.java:
##########
@@ -106,4 +106,14 @@ protected HoodieFileWriter newOrcFileWriter(
config.getLong(HoodieStorageConfig.ORC_FILE_MAX_SIZE), filter);
return new HoodieAvroOrcWriter(instantTime, path, orcConfig, schema,
taskContextSupplier);
}
+
+ private HoodieAvroWriteSupport getHoodieAvroWriteSupport(Configuration conf,
Schema schema,
+ HoodieConfig
config, boolean enableBloomFilter) {
+ Option<BloomFilter> filter = enableBloomFilter ?
Option.of(createBloomFilter(config)) : Option.empty();
+ HoodieAvroWriteSupport writeSupport = (HoodieAvroWriteSupport)
ReflectionUtils.loadClass(
Review Comment:
Wondering if we should introduce a new abstract class
`HoodieBaseAvroWriteSupport` (`HoodieAvroWriteSupport` will extend
`HoodieBaseAvroWriteSupport`) and use that here, instead of directly using
`HoodieAvroWriteSupport`. So other custom implementation may extend
`HoodieBaseAvroWriteSupport` or reuse part of `HoodieAvroWriteSupport` if
needed. Right now it has to extend `HoodieAvroWriteSupport`. @suryaprasanna
@vinothchandar wdyt?
##########
hudi-common/src/main/java/org/apache/hudi/common/config/HoodieStorageConfig.java:
##########
@@ -170,6 +170,12 @@ public class HoodieStorageConfig extends HoodieConfig {
.withDocumentation("Expected additional compression as records move from
log files to parquet. Used for merge_on_read "
+ "table to send inserts into log files & control the size of
compacted parquet file.");
+ public static final ConfigProperty<String> HOODIE_AVRO_WRITE_SUPPORT_CLASS =
ConfigProperty
+ .key("hoodie.avro.write.support.class")
+ .defaultValue("org.apache.hudi.avro.HoodieAvroWriteSupport")
+ .withDocumentation("Different write support classes can be loaded at
runtime. This is only required when trying to "
Review Comment:
Based on the logic, the class has to be a subclass of
`HoodieAvroWriteSupport`. Could you add the docs mentioning 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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
