[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-15 Thread Tim Armstrong (Code Review)
Tim Armstrong has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/14062 )

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..

IMPALA-8849: fix IllegalStateException with VARCHAR

The bug is that the serialized size wasn't populated
for VARCHAR in a case when it should have been.
It appears a condition was simply not updated when
VARCHAR was added.

Other code assumed that the serialized size was
populated when the other size field was populated,
which is a reasonable invariant. I documented the
invariant in the class and added validation that
the invariant held.

Defining and checking invariants led to discovering
various other minor issues where the sizes were
set incorrect for fixed-length types or not set for
variable-length types:
* CHAR was not consistently treated as a fixed-length type.
* avgSerializedSize_ was not always updated with avgSize_

Testing:
Added a regression test for this specific case. Adding
the assertions resulted in other cases showing up
related bugs.

Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Reviewed-on: http://gerrit.cloudera.org:8080/14062
Tested-by: Impala Public Jenkins 
Reviewed-by: Csaba Ringhofer 
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/bin/compute-table-stats.sh
M testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test
M testdata/workloads/functional-planner/queries/PlannerTest/empty.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
8 files changed, 154 insertions(+), 79 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Csaba Ringhofer: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 4
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 


[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-15 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14062 )

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..


Patch Set 3:

(1 comment)

Ah no problem, the issue was more that this class didn't have clear invariants 
and different methods assumed different things.

http://gerrit.cloudera.org:8080/#/c/14062/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test
File testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test:

http://gerrit.cloudera.org:8080/#/c/14062/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test@11
PS3, Line 11: HDFS
> What caused this change? I didn't find related codes in this patch.
It was added quite a while ago. The planner test validation ignores this 
prefix, so it only got added when I overwrite this whole file with the new 
results. See IMPALA-6050



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 15 Aug 2019 14:05:47 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-15 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14062 )

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..


Patch Set 3: Code-Review+2

(1 comment)

Thanks for fixing my mess in https://gerrit.cloudera.org/#/c/13190/ !

http://gerrit.cloudera.org:8080/#/c/14062/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test
File testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test:

http://gerrit.cloudera.org:8080/#/c/14062/3/testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test@11
PS3, Line 11: HDFS
What caused this change? I didn't find related codes in this patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Thu, 15 Aug 2019 13:39:18 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-14 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14062 )

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..


Patch Set 3: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 14 Aug 2019 23:41:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-14 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14062 )

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/4252/ : Initial code 
review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun 
to run full precommit tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 14 Aug 2019 20:18:00 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-14 Thread Tim Armstrong (Code Review)
Hello Impala Public Jenkins,

I'd like you to reexamine a change. Please visit

http://gerrit.cloudera.org:8080/14062

to look at the new patch set (#3).

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..

IMPALA-8849: fix IllegalStateException with VARCHAR

The bug is that the serialized size wasn't populated
for VARCHAR in a case when it should have been.
It appears a condition was simply not updated when
VARCHAR was added.

Other code assumed that the serialized size was
populated when the other size field was populated,
which is a reasonable invariant. I documented the
invariant in the class and added validation that
the invariant held.

Defining and checking invariants led to discovering
various other minor issues where the sizes were
set incorrect for fixed-length types or not set for
variable-length types:
* CHAR was not consistently treated as a fixed-length type.
* avgSerializedSize_ was not always updated with avgSize_

Testing:
Added a regression test for this specific case. Adding
the assertions resulted in other cases showing up
related bugs.

Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
---
M fe/src/main/java/org/apache/impala/analysis/AlterTableSetColumnStats.java
M fe/src/main/java/org/apache/impala/catalog/ColumnStats.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.java
M testdata/bin/compute-table-stats.sh
M testdata/workloads/functional-planner/queries/PlannerTest/card-scan.test
M testdata/workloads/functional-planner/queries/PlannerTest/empty.test
M 
testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test
M testdata/workloads/functional-query/queries/QueryTest/compute-stats.test
8 files changed, 154 insertions(+), 79 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/62/14062/3
--
To view, visit http://gerrit.cloudera.org:8080/14062
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 


[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR

2019-08-14 Thread Impala Public Jenkins (Code Review)
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14062 )

Change subject: IMPALA-8849: fix IllegalStateException with VARCHAR
..


Patch Set 3:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4790/ 
DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ie45e386cb09e31f4b7cdc82b7734dbecb4464534
Gerrit-Change-Number: 14062
Gerrit-PatchSet: 3
Gerrit-Owner: Tim Armstrong 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Comment-Date: Wed, 14 Aug 2019 19:36:26 +
Gerrit-HasComments: No