kasakrisz commented on code in PR #3289:
URL: https://github.com/apache/hive/pull/3289#discussion_r907347860
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/SimpleFetchOptimizer.java:
##########
@@ -152,28 +152,32 @@ private FetchTask optimize(ParseContext pctx, String
alias, TableScanOperator so
}
private boolean checkThreshold(FetchData data, int limit, ParseContext pctx)
throws Exception {
- if (limit > 0) {
- if (data.hasOnlyPruningFilter()) {
- /* partitioned table + query has only pruning filters */
- return true;
- } else if (data.isPartitioned() == false && data.isFiltered() == false) {
- /* unpartitioned table + no filters */
- return true;
+ boolean cachingEnabled = HiveConf.getBoolVar(pctx.getConf(),
HiveConf.ConfVars.HIVEFETCHTASKCACHING);
+ if (!cachingEnabled) {
+ /* if caching is enabled we apply the treshold in all cases */
Review Comment:
The `cachingEnabled` variable name and the comment suggest that true means
caching is enabled but the `if` predicate is negated. Does is means that this
branch should run when caching is disabled?
##########
ql/src/test/results/clientpositive/llap/nonmr_fetch_threshold.q.out:
##########
@@ -115,12 +115,15 @@ Plan optimized by CBO.
Stage-0
Fetch Operator
limit:10
- Select Operator [SEL_2]
- Output:["_col0","_col1","_col2","_col3"]
- Limit [LIM_3]
- Number of rows:10
- TableScan [TS_0]
- Output:["key","value"]
+ Stage-1
+ Map 1 vectorized, llap
+ File Output Operator [FS_8]
Review Comment:
It seems that this plan has a Map phase. Does this expected?
##########
ql/src/java/org/apache/hadoop/hive/ql/optimizer/SimpleFetchOptimizer.java:
##########
@@ -152,28 +152,32 @@ private FetchTask optimize(ParseContext pctx, String
alias, TableScanOperator so
}
private boolean checkThreshold(FetchData data, int limit, ParseContext pctx)
throws Exception {
- if (limit > 0) {
- if (data.hasOnlyPruningFilter()) {
- /* partitioned table + query has only pruning filters */
- return true;
- } else if (data.isPartitioned() == false && data.isFiltered() == false) {
- /* unpartitioned table + no filters */
- return true;
+ boolean cachingEnabled = HiveConf.getBoolVar(pctx.getConf(),
HiveConf.ConfVars.HIVEFETCHTASKCACHING);
+ if (!cachingEnabled) {
+ /* if caching is enabled we apply the treshold in all cases */
+ if (limit > 0) {
+ if (data.hasOnlyPruningFilter()) {
+ /* partitioned table + query has only pruning filters */
Review Comment:
nit: I thinks we should use `//` for one line comments.
--
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]
