[Impala-ASF-CR] IMPALA-8849: fix IllegalStateException with VARCHAR
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
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
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
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
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
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
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