impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Marcel Kornacker (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-4314: Standardize on MT-related data structures
Date Thu, 27 Oct 2016 16:23:19 GMT
Marcel Kornacker has posted comments on this change.

Change subject: IMPALA-4314: Standardize on MT-related data structures

Patch Set 2:

File be/src/runtime/

Line 457:   bool is_mt_execution = request.query_ctx.request.query_options.mt_dop > 0;
> set in FE?
didn't seem worth it. it would be a single bool instead of a single int with a '> 0' around
File be/src/scheduling/

Line 60:   // extract TPlanFragments and order by fragment id
> order by fragment idx?

Line 70:   DCHECK_EQ(fragment_exec_params_.size(), 0);
> comment that we asserting this is the first call to Init()

Line 244:   if (fragment->partition.type == TPartitionType::UNPARTITIONED) {
> Shouldn't this be a DCHECK instead? Every TStmtType::QUERY must have such a
but this should be callable for a non-query stmt (so the coordinator doesn't need to understand
at all times what it's dealing with).

Line 264:   const FragmentExecParams* fragment_params = &fragment_exec_params_[coord_fragment.idx];
> make this a FragmentExecParams&

Line 265:   DCHECK(fragment_params != nullptr);
> weird DCHECK, fix by using const& as suggested above
File be/src/scheduling/query-schedule.h:

Line 211:   // populated by Scheduler::Schedule (SimpleScheduler::ComputeFInstanceExecParams())
> now populated in Init() right?
File be/src/scheduling/

Line 725: #if 0
> ?

Line 750:     // TODO: we'll need to revisit this strategy once we can partition joins
> Maybe we should deal with this in the planner? I think the only way we can 
could think about that, but let's do that in a follow-on change.
File common/thrift/Frontend.thrift:

Line 351:   // The node ids refer to scan nodes in fragments[].plan_tree
> fragments[].plan (not plan_tree)
File fe/src/main/java/org/apache/impala/service/

Line 1000:     boolean disableSpilling = 
> whitespace

To view, visit
To unsubscribe, visit

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

View raw message