[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-10 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 4:

I'll promote to +2 once you decide what to do about Csaba's comments and update 
the tests.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Fri, 10 May 2019 18:35:17 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

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

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 4: Code-Review+1

(4 comments)

I have a few more suggestions for the layout, feel free to ignore them if you 
disagree or do not want to spend more time with this.

http://gerrit.cloudera.org:8080/#/c/13193/4/testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test:

http://gerrit.cloudera.org:8080/#/c/13193/4/testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test@28
PS4, Line 28: missing stats
The missing stats lead to different explain output, which will cause failed 
tests during the Jenkins build.


http://gerrit.cloudera.org:8080/#/c/13193/4/testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test@219
PS4, Line 219: Slot format=
Maybe write this only once before the first tuple?


http://gerrit.cloudera.org:8080/#/c/13193/4/testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test@220
PS4, Line 220:   0   1
I would prefer a smaller padding to make the lines more compact. I wouldn't 
care about very large offsets, e.g. above  - as the offsets are ordered, so 
most row will naturally aligned even without padding.


http://gerrit.cloudera.org:8080/#/c/13193/4/testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test@221
PS4, Line 221: ARRAY>   array_array_col
An idea to complicate the layout further: we could make the scalar case more 
compact + print the whole complex types by creating a new column like 
"complex-type" after the path. It would be empty for scalars, and the 3rd 
column for collections could be simply COLLECTION.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 08 May 2019 09:33:58 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

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

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 4:

Build Failed

https://jenkins.impala.io/job/gerrit-code-review-checks/3120/ : Initial code 
review checks failed. See linked job for details on the failure.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 08 May 2019 03:29:42 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-07 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 4: Code-Review+1

(1 comment)

I like the new layout.

http://gerrit.cloudera.org:8080/#/c/13193/4/fe/src/main/java/org/apache/impala/catalog/StructType.java
File fe/src/main/java/org/apache/impala/catalog/StructType.java:

http://gerrit.cloudera.org:8080/#/c/13193/4/fe/src/main/java/org/apache/impala/catalog/StructType.java@108
PS4, Line 108:   public String getExplainString(int depth) {
Could maybe do with a one-sentence comment explaining why we don't print 
contents.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Wed, 08 May 2019 00:33:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-07 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 3:

(10 comments)

I have addressed the review comments. Patch #4 makes the necessary code changes 
for proper explain layout. I have however only updated the "tuple-layout.test" 
for now. Changing all the tests is rather extensive and I want to first get 
input on the layout in patch #4. Once we agree on a proper layout I can make 
the changes in all test units.

http://gerrit.cloudera.org:8080/#/c/13193/3/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/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@76
PS3, Line 76:   public static final OffsetComparator OffsetComparator_ = new 
OffsetComparator();
> you can probably do:
Done


http://gerrit.cloudera.org:8080/#/c/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@331
PS3, Line 331:   expBuilder.append("Slot " + getId() + " ");
> Consider using Objects.toStringHelper, eg:
Decided to use a new format where we have a tuple header
{Tuple=, byteSize=, path=, Slot format=}

The slot field names which we were earlier printing for every slot are now only 
printed once per tuple in the tuple header.


http://gerrit.cloudera.org:8080/#/c/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@333
PS3, Line 333: getType
> Would be better to call toSql() or whatever other method explicitly instead
I have added a getExplainString() for the Type class.


http://gerrit.cloudera.org:8080/#/c/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@334
PS3, Line 334:   expBuilder.append("path=" + getPath() + " ");
> Would be better to call getExplainString() or whatever other method explici
Added Path::getExplainString().


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:

http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@78
PS3, Line 78: Tuple 0:
> Might be worth including the tuple byte size in the header? It's otherwise
Done


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@79
PS3, Line 79:   Slot 0 offset=0 
type=ARRAY>>>
 path=c.c_orders nullable=true
> As discussed offline, I think we want to avoid including the type of the ne
Nested/Complex Structs are now being represented as STRUCT<...>


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@80
PS3, Line 80:   Slot 2 offset=12 type=BIGINT path=c.c_custkey nullable=true
> Maybe we can drop the "Slot " prefix to make it a little denser? I don't
I ended up dropping Slot  and other filed names. The tuple header is also 
changed as follows:

Tuple= byteSize= path= Slot Format=

The "path.relative-path" returns the proper path for a slot. This was done 
because for most slots the path had a common prefix (TupleDescriptor's path)


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test:

http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test@53
PS3, Line 53:   Slot 10 offset=44 type=INT path=tpcds.store_sales.ss_quantity 
nullable=true
:   Slot 11 offset=48 type=DECIMAL(7,2) 
path=tpcds.store_sales.ss_wholesale_cost nullable=true
> These tables could be nicer to look at if the columns would be aligned with
Good point. I added padding for better readability and moved the repeated filed 
names to Tuple's header. Also moved path to the end. Please look at the new 
changes in "tuple-layout.test"


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test@55
PS3, Line 55: true
> Maybe replace true/false with 1/0 just to make it shorter?
Done


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test@56
PS3, Line 56: type=
> "type=" seems redundant to me - the SQL types with capital letters are reco
Done. Also the other field names were also redundant and so moved them to the 
Tuple's header.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: 

[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-07 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..

IMPALA-7293: Show tuple layout in explain plan.

This change prints materialized tuple's layout in the explain plan, at
explain_level >= 2.

New logic was added in the Planner::getExplainString(..) to print the
tuple layouts. The logic walks through the TupleDescriptors (and
SlotDescriptors) related to a given PlannerContext (for a given query),
stored in the associated global state (Analyzer.globalState_.descTbl
object) and prints materialized tuples and slots.

Testing:
- Tons of existing test units in PlannerTest were updated to include
  the tuple layout. The existing test cases provide good coverage.
- New testunit testTupleLayout was added to PlannerTest.java. The new
  test unit adds scenarios involving scalar types, complex types, joins,
  order-by, aggregation, union all and subqueries.

Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
---
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
M fe/src/main/java/org/apache/impala/analysis/Path.java
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/ArrayType.java
M fe/src/main/java/org/apache/impala/catalog/MapType.java
M fe/src/main/java/org/apache/impala/catalog/ScalarType.java
M fe/src/main/java/org/apache/impala/catalog/StructType.java
M fe/src/main/java/org/apache/impala/catalog/Type.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.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/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.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/tablesample.test
A testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
26 files changed, 4,932 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/93/13193/4
--
To view, visit http://gerrit.cloudera.org:8080/13193
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 4
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-02 Thread Todd Lipcon (Code Review)
Todd Lipcon has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/13193/3/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/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@76
PS3, Line 76:   public static final OffsetComparator OffsetComparator_ = new 
OffsetComparator();
you can probably do:

public static final Comparator COMPARE_BY_OFFSET =
  Ordering.natural().onResultOf(SlotDescriptor::getByteOffset);

(using guava's Ordering utility)


http://gerrit.cloudera.org:8080/#/c/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@331
PS3, Line 331:   expBuilder.append("Slot " + getId() + " ");
Consider using Objects.toStringHelper, eg:

return Objects.toStringHelper("Slot")
  .add("offset", getByteOffset())
  .add(type, getType()
  .toString();

which results in a string like 'Slot{offset=1, type=INT}', etc. and it has 
handy methods to skip null, etc.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Reviewer: Todd Lipcon 
Gerrit-Comment-Date: Fri, 03 May 2019 05:49:21 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-02 Thread Csaba Ringhofer (Code Review)
Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 3:

(3 comments)

The change looks good to me. The comments are about ideas to make the output 
more readable (for us, as I think that no one else than Impala developers and 
tests will read these lines). Our eyes will appreciate a good layout if we will 
read it a lot in the future. :)

http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test:

http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test@53
PS3, Line 53:   Slot 10 offset=44 type=INT path=tpcds.store_sales.ss_quantity 
nullable=true
:   Slot 11 offset=48 type=DECIMAL(7,2) 
path=tpcds.store_sales.ss_wholesale_cost nullable=true
These tables could be nicer to look at if the columns would be aligned with 
some padding. My suggestion is to move path to the last column, as it has the 
most variable length, and pad other columns to some same length, e.g. 2 digits 
for Slot id, 3 for offset and 14 for type. The main benefit would be that 
columns names from the same db + table would be always aligned.


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test@55
PS3, Line 55: true
Maybe replace true/false with 1/0 just to make it shorter?


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/fk-pk-join-detection.test@56
PS3, Line 56: type=
"type=" seems redundant to me - the SQL types with capital letters are 
recognizable enough.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Csaba Ringhofer 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Thu, 02 May 2019 23:18:44 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-05-01 Thread Tim Armstrong (Code Review)
Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 3:

(5 comments)

I'm at the point where I'd personally find this useful to understand the system 
and debug issues. I have a few suggestions for changes, some which we discussed 
offline before.

I'm not sure if others are likely to have a strong opinion - I feel like we can 
always iterate on it if it's missing things or people find things they don't 
like abou tit.

http://gerrit.cloudera.org:8080/#/c/13193/3/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/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@333
PS3, Line 333: getType
Would be better to call toSql() or whatever other method explicitly instead of 
relying on implicit toString(). Generally toString() returns debugging output, 
not output appropriate for end-users.


http://gerrit.cloudera.org:8080/#/c/13193/3/fe/src/main/java/org/apache/impala/analysis/SlotDescriptor.java@334
PS3, Line 334:   expBuilder.append("path=" + getPath() + " ");
Would be better to call getExplainString() or whatever other method explicitly 
instead of relying on implicit toString().


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test
File 
testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test:

http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@78
PS3, Line 78: Tuple 0:
Might be worth including the tuple byte size in the header? It's otherwise a 
little tricky to infer how big it is with padding and null bit on the end? Ok 
to ignore if it'll make it too noisy.


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@79
PS3, Line 79:   Slot 0 offset=0 
type=ARRAY>>>
 path=c.c_orders nullable=true
As discussed offline, I think we want to avoid including the type of the nested 
struct. It's noisy and doesn't match the physical representation (since fields 
may not be materialised).

It still makes sense to include types of nested arrays and maps.


http://gerrit.cloudera.org:8080/#/c/13193/3/testdata/workloads/functional-planner/queries/PlannerTest/constant-folding.test@80
PS3, Line 80:   Slot 2 offset=12 type=BIGINT path=c.c_custkey nullable=true
Maybe we can drop the "Slot " prefix to make it a little denser? I don't 
think the slot IDs are particularly useful.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 01 May 2019 22:43:16 +
Gerrit-HasComments: Yes


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

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

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 3:

Build Successful

https://jenkins.impala.io/job/gerrit-code-review-checks/3020/ : 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/13193
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 01 May 2019 01:09:51 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-04-30 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/13193 )

Change subject: IMPALA-7293: Show tuple layout in explain plan.
..


Patch Set 3:

fe tests ran clean: 
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/5447/console


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Abhishek Rawat 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong 
Gerrit-Comment-Date: Wed, 01 May 2019 00:11:27 +
Gerrit-HasComments: No


[Impala-ASF-CR] IMPALA-7293: Show tuple layout in explain plan.

2019-04-30 Thread Abhishek Rawat (Code Review)
Abhishek Rawat has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/13193


Change subject: IMPALA-7293: Show tuple layout in explain plan.
..

IMPALA-7293: Show tuple layout in explain plan.

This change prints materialized tuple's layout in the explain plan, at
explain_level >= 2.

New logic was added in the Planner::getExplainString(..) to print the
tuple layouts. The logic walks through the TupleDescriptors (and
SlotDescriptors) related to a given PlannerContext (for a given query),
stored in the associated global state (Analyzer.globalState_.descTbl
object) and prints materialized tuples and slots.

Testing:
- Tons of existing test units in PlannerTest were updated to include
  the tuple layout. The existing test cases provide good coverage.
- New testunit testTupleLayout was added to PlannerTest.java. The new
  test unit adds scenarios involving scalar types, complex types, joins,
  order-by, aggregation, union all and subqueries.

Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
---
M fe/src/main/java/org/apache/impala/analysis/DescriptorTable.java
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/Type.java
M fe/src/main/java/org/apache/impala/planner/Planner.java
M fe/src/test/java/org/apache/impala/planner/PlannerTest.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/kudu-selectivity.test
M testdata/workloads/functional-planner/queries/PlannerTest/max-row-size.test
M 
testdata/workloads/functional-planner/queries/PlannerTest/min-max-runtime-filters.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/tablesample.test
A testdata/workloads/functional-planner/queries/PlannerTest/tuple-layout.test
M testdata/workloads/functional-planner/queries/PlannerTest/union.test
21 files changed, 4,878 insertions(+), 0 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Iac38290217a219e75400e4206835992a7ea31dba
Gerrit-Change-Number: 13193
Gerrit-PatchSet: 3
Gerrit-Owner: Abhishek Rawat 
Gerrit-Reviewer: Impala Public Jenkins 
Gerrit-Reviewer: Tim Armstrong