You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org> on 2018/11/13 19:44:23 UTC
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Pooja Nilangekar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11599
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
IMPALA-7367: Pack StringValue and CollectionValue slots
This change packs StringValue and CollectionValue slots to ensure
they now occupy 12 bytes instead of 16 bytes. This reduces the
memory requirements and improves the performance. Since Kudu
tuples are populated using a memcopy, 4 bytes of padding was
added to StringSlots in Kudu tables.
Testing:
Ran core tests.
Added static asserts to ensure the value sizes are as expected.
Performance tests on TPCH-40 produced 3.96% improvement.
Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
---
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/runtime/collection-value.h
M be/src/runtime/descriptors.cc
M be/src/runtime/string-value.h
M be/src/runtime/types.h
M be/src/util/static-asserts.cc
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
30 files changed, 1,699 insertions(+), 1,707 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/11599/2
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1361/ : 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/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 01:46:07 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1393/ : 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/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Nov 2018 17:45:12 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
IMPALA-7367: Pack StringValue and CollectionValue slots
This change packs StringValue and CollectionValue slots to ensure
they now occupy 12 bytes instead of 16 bytes. This reduces the
memory requirements and improves the performance. Since Kudu
tuples are populated using a memcopy, 4 bytes of padding was
added to StringSlots in Kudu tables.
Testing:
Ran core tests.
Added static asserts to ensure the value sizes are as expected.
Performance tests on TPCH-40 produced 3.96% improvement.
Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
---
M be/src/exec/text-converter.inline.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/runtime/collection-value.h
M be/src/runtime/descriptors.cc
M be/src/runtime/string-value.h
M be/src/runtime/types.h
M be/src/util/static-asserts.cc
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
31 files changed, 1,700 insertions(+), 1,708 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/11599/3
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
IMPALA-7367: Pack StringValue and CollectionValue slots
This change packs StringValue and CollectionValue slots to ensure
they now occupy 12 bytes instead of 16 bytes. This reduces the
memory requirements and improves the performance. Since Kudu
tuples are populated using a memcopy, 4 bytes of padding was
added to StringSlots in Kudu tables.
Testing:
Ran core tests.
Added static asserts to ensure the value sizes are as expected.
Performance tests on TPCH-40 produced 3.96% improvement.
Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
---
M be/src/exec/text-converter.inline.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/runtime/collection-value.h
M be/src/runtime/descriptors.cc
M be/src/runtime/string-value.h
M be/src/runtime/types.h
M be/src/util/static-asserts.cc
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
32 files changed, 1,693 insertions(+), 1,818 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/11599/4
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 2:
> Patch Set 2:
>
> Build Failed
>
> https://jenkins.impala.io/job/gerrit-code-review-checks/1355/ : Initial code review checks failed. See linked job for details on the failure.
This change has been formatted and rebased. Seems like the failure is unrelated:
[exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd: can't cd to /docs/
[exec] tar: *gplcompression*: Cannot stat: No such file or directory
[exec] tar: Exiting with failure status due to previous errors
[exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd: can't cd to /hadoop-lzo-0.4.15.jar/
[exec] tar: *gplcompression*: Cannot stat: No such file or directory
[exec] tar: Exiting with failure status due to previous errors
[exec] tar: *gplcompression*: Cannot stat: No such file or directory
[exec] tar: Exiting with failure status due to previous errors
[copy] Copying 77 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/docs
[copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
[copy] Copying 4 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/ivy
[copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
[copy] Copying 64 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/src
[copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
Do I need to do something else to fix this?
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Nov 2018 21:21:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 6: Verified-1
Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3465/
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Fri, 16 Nov 2018 03:05:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
IMPALA-7367: Pack StringValue and CollectionValue slots
This change packs StringValue and CollectionValue slots to ensure
they now occupy 12 bytes instead of 16 bytes. This reduces the
memory requirements and improves the performance. Since Kudu
tuples are populated using a memcopy, 4 bytes of padding was
added to StringSlots in Kudu tables.
Testing:
Ran core tests.
Added static asserts to ensure the value sizes are as expected.
Performance tests on TPCH-40 produced 3.96% improvement.
Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Reviewed-on: http://gerrit.cloudera.org:8080/11599
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Tim Armstrong <ta...@cloudera.com>
---
M be/src/exec/text-converter.inline.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/runtime/collection-value.h
M be/src/runtime/descriptors.cc
M be/src/runtime/string-value.h
M be/src/runtime/types.h
M be/src/util/static-asserts.cc
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-no-debug-action.test
33 files changed, 1,625 insertions(+), 1,634 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Tim Armstrong: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 8
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 2:
Build Failed
https://jenkins.impala.io/job/gerrit-code-review-checks/1355/ : Initial code review checks failed. See linked job for details on the failure.
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Nov 2018 20:53:43 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3465/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 18:49:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Bikramjeet Vig (Code Review)" <ge...@cloudera.org>.
Bikramjeet Vig has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 4:
(2 comments)
looks good, I'll let Tim +2 after you have addressed his latest comment.
http://gerrit.cloudera.org:8080/#/c/11599/4/be/src/exec/text-converter.inline.h
File be/src/exec/text-converter.inline.h:
http://gerrit.cloudera.org:8080/#/c/11599/4/be/src/exec/text-converter.inline.h@86
PS4, Line 86: int str_len = str.len;
nit: can you still add a short comment here explaining the local variable, since its not immediately obvious why this workaround is necessary
http://gerrit.cloudera.org:8080/#/c/11599/4/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
File fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java:
http://gerrit.cloudera.org:8080/#/c/11599/4/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@186
PS4, Line 186: /VARCHAR
nit: I believe varchar is not supported in kudu
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 02:21:15 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
IMPALA-7367: Pack StringValue and CollectionValue slots
This change packs StringValue and CollectionValue slots to ensure
they now occupy 12 bytes instead of 16 bytes. This reduces the
memory requirements and improves the performance. Since Kudu
tuples are populated using a memcopy, 4 bytes of padding was
added to StringSlots in Kudu tables.
Testing:
Ran core tests.
Added static asserts to ensure the value sizes are as expected.
Performance tests on TPCH-40 produced 3.96% improvement.
Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
---
M be/src/exec/text-converter.inline.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/runtime/collection-value.h
M be/src/runtime/descriptors.cc
M be/src/runtime/string-value.h
M be/src/runtime/types.h
M be/src/util/static-asserts.cc
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
32 files changed, 1,704 insertions(+), 1,713 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/11599/5
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
IMPALA-7367: Pack StringValue and CollectionValue slots
This change packs StringValue and CollectionValue slots to ensure
they now occupy 12 bytes instead of 16 bytes. This reduces the
memory requirements and improves the performance. Since Kudu
tuples are populated using a memcopy, 4 bytes of padding was
added to StringSlots in Kudu tables.
Testing:
Ran core tests.
Added static asserts to ensure the value sizes are as expected.
Performance tests on TPCH-40 produced 3.96% improvement.
Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
---
M be/src/exec/text-converter.inline.h
M be/src/exprs/expr-test.cc
M be/src/exprs/scalar-expr.cc
M be/src/runtime/collection-value.h
M be/src/runtime/descriptors.cc
M be/src/runtime/string-value.h
M be/src/runtime/types.h
M be/src/util/static-asserts.cc
M fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java
M fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
M fe/src/main/java/org/apache/impala/catalog/PrimitiveType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzerTest.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
M testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering-disabled.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
M testdata/workloads/functional-planner/queries/PlannerTest/partition-pruning.test
M testdata/workloads/functional-planner/queries/PlannerTest/resource-requirements.test
M testdata/workloads/functional-planner/queries/PlannerTest/sort-expr-materialization.test
M testdata/workloads/functional-planner/queries/PlannerTest/spillable-buffer-sizing.test
M testdata/workloads/functional-planner/queries/PlannerTest/subquery-rewrite.test
M testdata/workloads/functional-planner/queries/PlannerTest/tablesample.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpcds-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-all.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-kudu.test
M testdata/workloads/functional-planner/queries/PlannerTest/tpch-nested.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level2.test
M testdata/workloads/functional-query/queries/QueryTest/explain-level3.test
M testdata/workloads/functional-query/queries/QueryTest/spilling-no-debug-action.test
33 files changed, 1,625 insertions(+), 1,634 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/11599/7
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 6
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 18:49:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1371/ : 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/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 18:48:35 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 2:
> Patch Set 2:
>
> > Patch Set 2:
> >
> > Build Failed
> >
> > https://jenkins.impala.io/job/gerrit-code-review-checks/1355/ : Initial code review checks failed. See linked job for details on the failure.
>
> This change has been formatted and rebased. Seems like the failure is unrelated:
>
> [exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd: can't cd to /docs/
> [exec] tar: *gplcompression*: Cannot stat: No such file or directory
> [exec] tar: Exiting with failure status due to previous errors
> [exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd: can't cd to /hadoop-lzo-0.4.15.jar/
> [exec] tar: *gplcompression*: Cannot stat: No such file or directory
> [exec] tar: Exiting with failure status due to previous errors
> [exec] tar: *gplcompression*: Cannot stat: No such file or directory
> [exec] tar: Exiting with failure status due to previous errors
> [copy] Copying 77 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/docs
> [copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
> [copy] Copying 4 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/ivy
> [copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
> [copy] Copying 64 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/src
> [copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
>
>
> Do I need to do something else to fix this?
This seems to be the actual errors: https://jenkins.impala.io/job/clang-tidy-ub1604/4142/artifact/tidylog.txt
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 2
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Nov 2018 21:26:01 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 5: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 5
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 18:40:38 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 3:
(9 comments)
Looking good, mainly had minor comments.
http://gerrit.cloudera.org:8080/#/c/11599/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/11599/3//COMMIT_MSG@18
PS3, Line 18: Performance tests on TPCH-40 produced 3.96% improvement.
I'm curious why this is so different from your earlier results on scale factor 60 - was there an issue with the previous results? Or does the larger scale factor or different system make a big difference?
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exec/text-converter.inline.h
File be/src/exec/text-converter.inline.h:
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exec/text-converter.inline.h@86
PS3, Line 86: UnescapeString(data, str.ptr, &str.len, buffer_len); // NOLINT
Can you add a brief comment explaining the NOLINT. Or maybe there's a reasonable way to work around it like using a temporary value on the stack?
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exprs/expr-test.cc
File be/src/exprs/expr-test.cc:
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exprs/expr-test.cc@7610
PS3, Line 7610: // Test layout adding a bunch of exprs. This is designed to trigger padding.
Update comment - maybe mention that it was padded in the past but that now it isn't
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exprs/expr-test.cc@7656
PS3, Line 7656: // ASSERT_EQ(expected_byte_size % 8, 0);
Remove?
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exprs/scalar-expr.cc
File be/src/exprs/scalar-expr.cc:
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exprs/scalar-expr.cc@228
PS3, Line 228: // TODO: sort by type as well? Any reason to do this?
Remove these TODOs?
http://gerrit.cloudera.org:8080/#/c/11599/3/be/src/exprs/scalar-expr.cc@262
PS3, Line 262: // Walk the types and store in a packed aligned layout
Stale comment?
http://gerrit.cloudera.org:8080/#/c/11599/3/fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java
File fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java:
http://gerrit.cloudera.org:8080/#/c/11599/3/fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java@275
PS3, Line 275:
nit: unnecessary/inconsistent vertical whitespace
http://gerrit.cloudera.org:8080/#/c/11599/3/fe/src/main/java/org/apache/impala/analysis/TupleDescriptor.java@347
PS3, Line 347: private boolean isKuduStringSlot(SlotDescriptor slot) {
Wouldn't it make more sense for this to be a SlotDescriptor method? It looks like SlotDescriptor has a reference to the parent TupleDescriptor.
http://gerrit.cloudera.org:8080/#/c/11599/3/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
File testdata/workloads/functional-planner/queries/PlannerTest/join-order.test:
http://gerrit.cloudera.org:8080/#/c/11599/3/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test@546
PS3, Line 546: # the largest input is prevented from becoming the leftmost input by the full outer join
This and the similar one below are outdated, I think from a time when the planner didn't invert some joins - maybe just remove them?
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 01:21:31 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/1368/ : 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/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 22:12:34 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 3:
> Patch Set 2:
>
> > Patch Set 2:
> >
> > > Patch Set 2:
> > >
> > > Build Failed
> > >
> > > https://jenkins.impala.io/job/gerrit-code-review-checks/1355/ : Initial code review checks failed. See linked job for details on the failure.
> >
> > This change has been formatted and rebased. Seems like the failure is unrelated:
> >
> > [exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd: can't cd to /docs/
> > [exec] tar: *gplcompression*: Cannot stat: No such file or directory
> > [exec] tar: Exiting with failure status due to previous errors
> > [exec] /home/ubuntu/hadoop-lzo/src/native/packageNativeHadoop.sh: 44: cd: can't cd to /hadoop-lzo-0.4.15.jar/
> > [exec] tar: *gplcompression*: Cannot stat: No such file or directory
> > [exec] tar: Exiting with failure status due to previous errors
> > [exec] tar: *gplcompression*: Cannot stat: No such file or directory
> > [exec] tar: Exiting with failure status due to previous errors
> > [copy] Copying 77 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/docs
> > [copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
> > [copy] Copying 4 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/ivy
> > [copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
> > [copy] Copying 64 files to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15/src
> > [copy] Copying 1 file to /home/ubuntu/hadoop-lzo/build/hadoop-lzo-0.4.15
> >
> >
> > Do I need to do something else to fix this?
>
> This seems to be the actual errors: https://jenkins.impala.io/job/clang-tidy-ub1604/4142/artifact/tidylog.txt
Thanks Fredy!
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 3
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Wed, 14 Nov 2018 00:55:26 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Tim Armstrong (Code Review)" <ge...@cloudera.org>.
Tim Armstrong has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/11599/3/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test
File testdata/workloads/functional-planner/queries/PlannerTest/join-order.test:
http://gerrit.cloudera.org:8080/#/c/11599/3/testdata/workloads/functional-planner/queries/PlannerTest/join-order.test@546
PS3, Line 546: # order does not become the leftmost input because of the outer join;
> Done
Sorry to be unclear - I think we should keep the test (it's useful to be able to look at the history and see how the plan changed over time) but remove the comment.
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 4
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Thu, 15 Nov 2018 00:59:52 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7367: Pack StringValue and CollectionValue slots
Posted by "Pooja Nilangekar (Code Review)" <ge...@cloudera.org>.
Pooja Nilangekar has posted comments on this change. ( http://gerrit.cloudera.org:8080/11599 )
Change subject: IMPALA-7367: Pack StringValue and CollectionValue slots
......................................................................
Patch Set 7:
> Patch Set 7:
>
> I can submit once you publish the draft.
Done.
--
To view, visit http://gerrit.cloudera.org:8080/11599
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I32f3b06622c087e4aa288e8db1bf4581b10d386a
Gerrit-Change-Number: 11599
Gerrit-PatchSet: 7
Gerrit-Owner: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Bikramjeet Vig <bi...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Jim Apple <jb...@apache.org>
Gerrit-Reviewer: Pooja Nilangekar <po...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <ta...@cloudera.com>
Gerrit-Comment-Date: Mon, 19 Nov 2018 17:12:50 +0000
Gerrit-HasComments: No