dongjoon-hyun commented on a change in pull request #936:
URL: https://github.com/apache/orc/pull/936#discussion_r727580287
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -176,6 +176,15 @@
+ "must have the filter\n"
+ "reapplied to avoid using unset values in the
unselected rows.\n"
+ "If unsure please leave this as false."),
+ ALLOW_PLUGIN_FILTER("orc.filter.plugin.enable",
+ "orc.filter.plugin.enable",
Review comment:
Thank you, @pavibhai . BTW, Apache ORC configuration convention doesn't
use `.enable` postfix, does it?
##########
File path: java/core/src/java/org/apache/orc/OrcConf.java
##########
@@ -176,6 +176,15 @@
+ "must have the filter\n"
+ "reapplied to avoid using unset values in the
unselected rows.\n"
+ "If unsure please leave this as false."),
+ ALLOW_PLUGIN_FILTER("orc.filter.plugin.enable",
+ "orc.filter.plugin.enable",
+ false,
+ "Enables the use of plugin filters during read. The
plugin filters "
+ + "are discovered against the service "
+ + "org.apache.orc.filter.PluginFilterService, if
multiple filters are "
+ + "determined they are combined using AND. The order of
application is "
Review comment:
`determined they` -> `determined, they`?
##########
File path: java/core/src/java/org/apache/orc/Reader.java
##########
@@ -637,6 +639,15 @@ public Options useSelected(boolean newValue) {
this.useSelected = newValue;
return this;
}
+
+ public boolean isAllowPluginFilters() {
Review comment:
`isAllow` sounds a little strange to me. Could you revise this method
name?
##########
File path: java/core/src/java/org/apache/orc/impl/filter/FilterFactory.java
##########
@@ -46,16 +49,35 @@
* @param isSchemaCaseAware identifies if the schema is case-sensitive
* @param version provides the ORC file version
* @param normalize identifies if the SArg should be normalized or
not
+ * @param filePath that is fully qualified to determine plugin filter(s)
+ * @param conf configuration shared when determining Plugin filter(s)
Review comment:
Could you align these new parameters with the other existing ones?
##########
File path: java/core/src/java/org/apache/orc/impl/filter/FilterFactory.java
##########
@@ -74,11 +96,6 @@ public static BatchFilter createBatchFilter(Reader.Options
opts,
}
}
- // 2. Process input filter
- if (opts.getFilterCallback() != null) {
Review comment:
Oh, do we need to move this?
--
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]
