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=<offset nullable type relative-path>
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 9999 - 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<INT>>               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 <ara...@cloudera.com>
Gerrit-Reviewer: Abhishek Rawat <ara...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <t...@apache.org>
Gerrit-Comment-Date: Wed, 08 May 2019 09:33:58 +0000
Gerrit-HasComments: Yes

Reply via email to