You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fredy Wijaya (Code Review)" <ge...@cloudera.org> on 2018/02/06 16:46:32 UTC
[Impala-ASF-CR] IMPALA-6392: Explain format for parquet predicate statistics should be consistent with predicates
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/9223
Change subject: IMPALA-6392: Explain format for parquet predicate statistics should be consistent with predicates
......................................................................
IMPALA-6392: Explain format for parquet predicate statistics should be
consistent with predicates
Change the explain format for parquet predicate statistics to output
each tuple descriptor per line. This change is to make it consistent
with the output of other predicates.
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 50 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/2
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
- Ran end-to-end tests in query_test
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 49 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/7
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 49 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/5
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
- Ran end-to-end tests in query_test
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 50 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/9
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
- Ran end-to-end tests in query_test
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 51 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/11
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 7: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/9223/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/7/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@230
PS7, Line 230: max
nit: lets keep this as minMax (as before), and consistent with the rest of the naming in this change.
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 21:23:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
- Ran end-to-end tests in query_test
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Reviewed-on: http://gerrit.cloudera.org:8080/9223
Tested-by: Impala Public Jenkins
Reviewed-by: Alex Behm <al...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 51 insertions(+), 18 deletions(-)
Approvals:
Impala Public Jenkins: Verified
Alex Behm: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 12: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 12
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 22:07:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
- Ran end-to-end tests in query_test
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 49 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/8
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 11: Code-Review+1
Carrying Vuk's +1
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 02:55:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9223/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9223/5//COMMIT_MSG@23
PS5, Line 23: .
> run end-to-end tests as well. see testdata/workloads/functional-query/queri
Done
http://gerrit.cloudera.org:8080/#/c/9223/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@230
PS5, Line 230: maxOrigina
> what's the "dictionary" prefix for? seems this is still just for min/max co
I was referring to a Map as a dictionary, but I'll change it to minMaxOriginalConjuncts instead.
http://gerrit.cloudera.org:8080/#/c/9223/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1100
PS5, Line 1100: MinMaxOrig
> make this name consistent with member name if updated.
Done
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 20:34:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
IMPALA-6392: Consistent explain format for parquet predicate statistics
In EXPLAIN_LEVEL=2+, change the explain format for parquet predicate
statistics to output each tuple descriptor per line. This change is to
make it consistent with the output of other predicates.
Before:
parquet statistics predicates: c_custkey < 10, o_orderkey < 5, l_linenumber < 3
After:
parquet statistics predicates: c_custkey < 10
parquet statistics predicates on o: o_orderkey < 5
parquet statistics predicates on o_lineitems: l_linenumber < 3
Testing:
- Ran existing planner tests and updated the ones that are affected by
this change.
- Ran end-to-end tests in query_test
Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
---
M fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
M testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
M testdata/workloads/functional-planner/queries/PlannerTest/mt-dop-validation.test
M testdata/workloads/functional-planner/queries/PlannerTest/parquet-filtering.test
4 files changed, 51 insertions(+), 18 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/23/9223/10
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 8:
(2 comments)
Nice!
http://gerrit.cloudera.org:8080/#/c/9223/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@523
PS8, Line 523: if (!minMaxOriginalConjuncts_.containsKey(tupleDescriptor)) {
Use this pattern for fewer lookups:
List l = map.get(key);
if (l == null) {
l = new List();
map.put(key, l);
}
l.add(value);
http://gerrit.cloudera.org:8080/#/c/9223/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1104
PS8, Line 1104: TupleDescriptor tupleDescriptor = entry.getKey();
style: we use these standard abbreviations in many places:
tupleDescriptor -> tupleDesc or td
expressions -> exprs
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 00:06:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 13: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:00:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 11: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 11
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 18:47:27 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Explain format for parquet predicate statistics should be consistent with predicates
Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Explain format for parquet predicate statistics should be consistent with predicates
......................................................................
Patch Set 2:
(9 comments)
Thanks for working on this!
Do you have a username on JIRA so that I can assign this (and the other ones you're working on) to you?
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-6392: Explain format for parquet predicate statistics should be
: consistent with predicates
shorten this to fit on one line
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@10
PS2, Line 10: explain format
mention that this only affects EXPLAIN_LEVEL=2+
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@13
PS2, Line 13:
Could you include a couple lines showing what the new format looks like?
Also, we like to include a "Testing" section. You can look through other reviews on gerrit to see what this normally looks like, but it would be something like:
Testing:
- Ran existing planner tests and updated the ones that are affected by this change.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@230
PS2, Line 230:
extra whitespace
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@232
PS2, Line 232:
extra whitespace
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1099
PS2, Line 1099: private String getDictionaryMinMaxOriginalConjunctsExplainString(String prefix) {
Brief comment here, like the one for getDictionaryConjunctsExplainString() below
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1107
PS2, Line 1107: prefix
This can fit on the previous line (we wrap at 90)
You might consider looking into clang-format-diff to find these little issues. We include a .clang-format file
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1109
PS2, Line 1109: String alias
Since this is only used once, no need to make a variable, just include it inline in the append()
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1111
PS2, Line 1111: prefix
fits on previous line
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 18:26:25 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 9: Code-Review+1
(2 comments)
Carrying Vuk's +1
http://gerrit.cloudera.org:8080/#/c/9223/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@523
PS8, Line 523: List<Expr> exprs = minMaxOriginalConjuncts_.get(tupleDesc);
> Use this pattern for fewer lookups:
Done
http://gerrit.cloudera.org:8080/#/c/9223/8/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1104
PS8, Line 1104: minMaxOriginalConjuncts_.entrySet()) {
> style: we use these standard abbreviations in many places:
Done
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 9
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 02:41:33 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6392: Explain format for parquet predicate statistics should be consistent with predicates
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Explain format for parquet predicate statistics should be consistent with predicates
......................................................................
Patch Set 2:
My username is fredyw
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 18:44:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Alex Behm (Code Review)" <ge...@cloudera.org>.
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 13: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Feb 2018 21:02:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 10: Code-Review+1
Carrying Vuk's +1
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 10
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Wed, 07 Feb 2018 02:44:53 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 13:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/1928/
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Alex Behm <al...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Feb 2018 17:16:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Vuk Ercegovac (Code Review)" <ge...@cloudera.org>.
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 5:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9223/5//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9223/5//COMMIT_MSG@23
PS5, Line 23: .
run end-to-end tests as well. see testdata/workloads/functional-query/queries/QueryTest/parquet-filtering.test. this is used by tests/query_test/test_mt_dop.py
http://gerrit.cloudera.org:8080/#/c/9223/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@230
PS5, Line 230: dictionary
what's the "dictionary" prefix for? seems this is still just for min/max conjuncts (minMaxConjuncts_) and not for dictionaryFilterConjuncts_.
http://gerrit.cloudera.org:8080/#/c/9223/5/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1100
PS5, Line 1100: Dictionary
make this name consistent with member name if updated.
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 19:27:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 8: Code-Review+1
> (1 comment)
carrying vuk's +1
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 8
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Reviewer: Vuk Ercegovac <ve...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 21:40:55 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6392: Consistent explain format for parquet predicate statistics
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9223 )
Change subject: IMPALA-6392: Consistent explain format for parquet predicate statistics
......................................................................
Patch Set 5:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@7
PS2, Line 7: IMPALA-6392: Consistent explain format for parquet predicate statistics
:
> shorten this to fit on one line
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@10
PS2, Line 10: to output each
> mention that this only affects EXPLAIN_LEVEL=2+
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2//COMMIT_MSG@13
PS2, Line 13: Before:
> Could you include a couple lines showing what the new format looks like?
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java
File fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java:
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@230
PS2, Line 230: M
> extra whitespace
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@232
PS2, Line 232:
> extra whitespace
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1099
PS2, Line 1099: // Helper method that prints dictionary min max original conjucts by tuple descriptor.
> Brief comment here, like the one for getDictionaryConjunctsExplainString()
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1107
PS2, Line 1107: ut.app
> This can fit on the previous line (we wrap at 90)
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1109
PS2, Line 1109: else {
> Since this is only used once, no need to make a variable, just include it i
Done.
http://gerrit.cloudera.org:8080/#/c/9223/2/fe/src/main/java/org/apache/impala/planner/HdfsScanNode.java@1111
PS2, Line 1111: prefix
> fits on previous line
The previous line will be 91-width long if we move prefix to the previous line.
--
To view, visit http://gerrit.cloudera.org:8080/9223
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ia3d55ab6a1ae551867a9f68b3622844102cc854e
Gerrit-Change-Number: 9223
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 06 Feb 2018 18:59:25 +0000
Gerrit-HasComments: Yes