Copilot commented on code in PR #6815:
URL: https://github.com/apache/hive/pull/6815#discussion_r4345298391
##########
ql/src/java/org/apache/hadoop/hive/ql/io/orc/ExternalCache.java:
##########
@@ -98,8 +98,7 @@ public boolean hasPpd() {
public void configure(HiveConf queryConfig) {
this.conf = queryConfig;
this.sarg = ConvertAstToSearchArg.createFromConf(conf);
- this.isPpdEnabled = HiveConf.getBoolVar(conf,
ConfVars.HIVE_OPT_INDEX_FILTER)
- && HiveConf.getBoolVar(conf, ConfVars.HIVE_ORC_MS_FOOTER_CACHE_PPD);
+ this.isPpdEnabled = HiveConf.getBoolVar(conf,
ConfVars.HIVE_ORC_MS_FOOTER_CACHE_PPD);
Review Comment:
With this change, `isPpdEnabled` no longer reflects whether query-level
predicate push-down is enabled (e.g., `hive.optimize.ppd`). This can cause ORC
footer-cache PPD to remain active even when a user explicitly disables PPD for
a query, which is a behavior change and can violate configuration expectations.
Consider gating `isPpdEnabled` on both `HIVE_ORC_MS_FOOTER_CACHE_PPD` and
`HIVE_OPT_PPD` so disabling `hive.optimize.ppd` reliably disables all PPD paths.
##########
common/src/java/org/apache/hadoop/hive/conf/HiveConf.java:
##########
@@ -2486,8 +2486,6 @@ public static enum ConfVars {
HIVE_SCRIPT_OPERATOR_TRUST("hive.exec.script.trust", false, ""),
// Optimizer
- HIVE_OPT_INDEX_FILTER("hive.optimize.index.filter", true, "Whether to
enable automatic use of indexes"),
-
HIVE_OPT_PPD("hive.optimize.ppd", true,
"Whether to enable predicate pushdown"),
Review Comment:
This PR removes a user-facing configuration key; the PR description notes
that with `hive.conf.validation=true` the removed key should now raise an
unknown-config error. Consider adding a dedicated negative qtest (e.g., under
`ql/src/test/queries/clientnegative`) that executes `set
hive.optimize.index.filter=false;` and asserts the expected unknown-config
failure. This will prevent regressions where the removed key is silently
accepted again.
##########
ql/src/java/org/apache/hadoop/hive/ql/io/parquet/LeafFilterFactory.java:
##########
@@ -47,7 +47,7 @@ public class LeafFilterFactory {
@VisibleForTesting
public static final String FILTER_PREDICATE_CONVERSION_NOT_SUPPORTED =
- "The conversion to Parquet FilterPredicate is not supported for %s.
Please try to set the following configurations at the session level\n set
hive.optimize.index.filter=false;\n"
+ "The conversion to Parquet FilterPredicate is not supported for %s.
Please try to set the following configuration at the session level\n"
+ " set hive.optimize.ppd=false;\n";
Review Comment:
The revised message is clearer than before, but it still reads tentative
(\"Please try\") and formats the recommended setting as a separate, indented
line. Consider rephrasing to a direct instruction (e.g., \"Disable predicate
pushdown by setting hive.optimize.ppd=false\") and removing the leading space
before `set` so the message is consistent and easy to copy/paste from logs.
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
