impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5483: Automatically disable codegen for small queries
Date Tue, 27 Jun 2017 00:05:03 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-5483: Automatically disable codegen for small queries
......................................................................


Patch Set 6:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/7153/6/testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test
File testdata/workloads/functional-planner/queries/PlannerTest/disable-codegen.test:

Line 6: Planner Hints: Disable codegen
> The term "hint" already has a different meaning as in "straight_join" kind 
As mentioned in the other comment the codegen hint isn't equivalent to the query option. It
seems a bit questionable that with the other query options we can't distinguish between user-set
options and planner-set options in the profile (although you can usually infer for single
node plan).

Disabling codegen does seem like a significant enough planner decision that it should be somewhat
visible in the explain plan. It's a bit less of a problem for the single node optimisation
since then there's usually a visible change to the plan shape. We can test it like testComputeStatsMtDop()
but if there's nothing in the explain plan it's harder to understand what the planner did.

"Planner Hint" might not be the best term if we do include it. Not sure what would be better.
Just a plain string like "Codegen disabled by planner"?


http://gerrit.cloudera.org:8080/#/c/7153/3/testdata/workloads/functional-query/queries/QueryTest/codegen.test
File testdata/workloads/functional-query/queries/QueryTest/codegen.test:

Line 2: ---- QUERY
> The hint is applied to all FE->BE expr evaluations, and the single-node opt
I think we have coverage that the query produces the expected results when the hint is set,
but we don't have any coverage that codegen was actually disabled.

The distinction between the hint and the query option exists for a reason - there are some
exprs that can only be evaluated with codegen enabled. In those cases the hint is overridden.
But if the query option is set explicitly, then the option takes precedence and the query
fails because the exprs can't be evaluated.

b7eeb8bf85ef24b730c9ba2891f5a8075ba9605e added some coverage of that interaction - it runs
test_udfs with exec_single_node_rows_threshold set to a couple of different values, which
exercises that case.


-- 
To view, visit http://gerrit.cloudera.org:8080/7153
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I273bcee58641f5b97de52c0b2caab043c914b32e
Gerrit-PatchSet: 6
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Juan Yu <jyu@cloudera.com>
Gerrit-Reviewer: Michael Ho
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Mostafa Mokhtar <mmokhtar@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message