[jira] [Commented] (IMPALA-8023) Fix PlannerTest to handle error lines consistently

["ASF subversion and git services (JIRA)" <ji...@apache.org>]

    [ https://issues.apache.org/jira/browse/IMPALA-8023?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16741351#comment-16741351 ] 

ASF subversion and git services commented on IMPALA-8023:
---------------------------------------------------------

Commit a7ea86b768247ff5388174445e7c91736b99c2de in impala's branch refs/heads/master from paul-rogers
[ https://git-wip-us.apache.org/repos/asf?p=impala.git;h=a7ea86b ]

IMPALA-8021: Add estimated cardinality to EXPLAIN output

Cardinality is vital to understanding why a plan has the form it does,
yet the planner normally emits cardinality information only for the
detailed levels. Unfortunately, most query profiles we see are at the
standard level without this information (except in the summary table),
making it hard to understand what happened.

This patch adds cardinality to the standard EXPLAIN output. It also
changes the displayed cardinality value to be in abbreviated "metric"
form: 1.23K instead of 1234, etc.

Changing the DESCRIBE output has a huge impact on PlannerTest: all the
"golden" test files must change. To avoid doing this twice, this patch
also includes:

IMPALA-7919: Add predicates line in plan output for partition key
predicates

This is also the time to also include:

IMPALA-8022: Add cardinality checks to PlannerTest

The comparison code was changed to allow a set of validators, one of
which compares cardinality to ensure it is within 5% of the expected
value. This should ensure we don't change estimates unintentionally.

While many planner tests are concerned with cardinality, many others are
not. Testing showed that the cardinality is actually unstable within
tests. For such tests, added filters to ignore cardinality. The filter
is enabled by default (for backward compatibility) but disabled (to
allow cardinality verification) for the critical tests.

Rebasing the tests was complicated by a bug in the error-matching code,
so this patch also fixes:

IMPALA-8023: Fix PlannerTest to handle error lines consistently

Now, the error output written to the output "save results" file matches
that expected in the "golden" file -- no more handling these specially.

Testing:

* Added cardinality verification.
* Reran all FE tests.
* Rebased all PlannerTest .test files.
* Adjusted the metadata/test_explain.py test to handle the changed
  EXPLAIN output.

Change-Id: Ie9aa2d715b04cbb279aaffec8c5692686562d986
Reviewed-on: http://gerrit.cloudera.org:8080/12136
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>


> Fix PlannerTest to handle error lines consistently
> --------------------------------------------------
>
>                 Key: IMPALA-8023
>                 URL: https://issues.apache.org/jira/browse/IMPALA-8023
>             Project: IMPALA
>          Issue Type: Improvement
>          Components: Frontend
>    Affects Versions: Impala 3.1.0
>            Reporter: Paul Rogers
>            Assignee: Paul Rogers
>            Priority: Minor
>
> {{PlannerTest}} works by running a query from a .test file, generating a plan, and comparing that plan to a "golden" expected result. It work well for most cases. We can use Eclipse's diff tools to compare the actual with expected files, and to copy across any expected changes that result from changes to the planner code.
> Once case that does *not* work are exceptions. When PlannerTest indicates encounters failure, it emits a line such as the following to the actual results file:
> {noformat}
> org.apache.impala.common.NotImplementedException: Scan of table 't' in format 'RC_FILE' is not supported because the table has a column 's' with a complex type 'STRUCT<f1:STRING,f2:INT>'.
> {noformat}
> Yet, in order for the comparison to pass, the golden file must contain the error in the following form:
> {noformat}
> NotImplementedException: Scan of table 'functional.complextypes_fileformat' in format 'TEXT' is not supported because the table has a column 's' with a complex type 'STRUCT<f1:STRING,f2:INT>'.
> {noformat}
> Note that the actual output includes the package prefix, the expected error must *not* include that prefix.
> The result is that:
> * When comparing files, one must learn to ignore the differences between these lines: the differences are *not* the reason why a test might fail, and
> * When "rebasing" a file, one must copy all expected changes *except* the error lines.
> In short, this is a real nuisance. Use a filter mechanism to fix this once and for all.
> The problem is that the text appended to the "actual output" is not the same as that used for comparison. A simple two-line fix will eliminate this issue.
> Current code in {{PlanerTestBase.handleException()}}:
> {code:java}
>     actualOutput.append(e.toString() + "\n");
> ...
>         String actualErrorMsg = e.getClass().getSimpleName() + ": " + e.getMessage();
> {code}
> Proposed:
> {code:java}
>     String actualErrorMsg = e.getClass().getSimpleName() + ": " + e.getMessage();
>     actualOutput.append(actualErrorMsg).append("\n");
> {code}



--
This message was sent by Atlassian JIRA
(v7.6.3#76005)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org