Alex Behm has posted comments on this change.

Change subject: IMPALA-4270: Gracefully fail unsupported queries with mt_dop > 

Patch Set 2:

File be/src/exec/

Line 272:           DCHECK_EQ(state->query_options().num_scanner_threads, 1);
> roll into a single dcheck with a compound predicate
File common/thrift/PlanNodes.thrift:

Line 205:   
> trailing

Line 207:   // If this is true then the MT_DOP query option must be > 0.
> leave todo that it should be removed later
File fe/src/main/java/org/apache/impala/planner/

Line 314:    * Retrurns a set of file formats being scanned.
> spelling

Line 562:     msg.hdfs_scan_node.setUse_mt_scan_nodeIsSet(useMtScanNode_);
> ..IsSet? it doesn't look like you're calling the actual setter
Weird. That seemed to work also. Fixed.
File fe/src/main/java/org/apache/impala/planner/

Line 206:     if (ctx_.getQueryOptions().mt_dop > 0) {
> the if is redundant, but you can make it a checkstate at the start of the f
Added Preconditions check.
File fe/src/main/java/org/apache/impala/planner/

Line 83:   public TQueryOptions getQueryOptions() { return 
getRootAnalyzer().getQueryOptions(); }
> why the change?
I added a convenience function Analyzer.getQueryOptions() and thought it would 
make sense to use it here.
File fe/src/main/java/org/apache/impala/planner/

Line 155:    * - MT_DOP > 0 is not supported for plans with distributed joins 
or table sinks.
> it's only supported for QueryStmts. take a look at Frontend.createExecReque
My thinking was that from a user's perspective running a statement with mt_dop 
> 0 should have one of two outcomes:
1. The statement runs fine in multi-threaded mode.
2. The statement returns an error indicating that mt_dop is not supported for 
that statement.

If that's the behavior we want, then it's irrelevant whether we can actually 
generate parallel plans for inserts/ctas.

Line 51: |  tuple-ids=0,1 row-size=8B cardinality=unavailable
> what happened here?
functional_parquet has no stats.

I preferred to write the tests on Parquet because that's the  preferred file 
format, happy to change it back or add more tests?

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I91a60ea7b6e3ae4ee44be856615ddd3cd0af476d
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Alex Behm <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Marcel Kornacker <>
Gerrit-HasComments: Yes

Reply via email to