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 <[email protected]> Gerrit-Reviewer: Abhishek Rawat <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Tim Armstrong <[email protected]> Gerrit-Comment-Date: Thu, 02 May 2019 23:18:44 +0000 Gerrit-HasComments: Yes
