You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Surya Hebbar (Code Review)" <ge...@cloudera.org> on 2023/04/14 14:06:36 UTC
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Surya Hebbar has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19745
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
---
M www/query_timeline.tmpl
1 file changed, 152 insertions(+), 95 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/19745/1
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 1
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12862/ : 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/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 13:47:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 7
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 17:24:15 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@100
PS4, Line 100: =
nit: add space around =
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@495
PS4, Line 495: x + px_per_tic - decimals * 6
Why is "0.00" shown on the right side of the first box? "0.00" as starting time should be shown on the left end of the first box. Maybe show two numbers in last box on each end so that the ending time is shown on the right side of the last box.
nit: remove space before ','
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 May 2023 22:02:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
---
M www/query_timeline.tmpl
1 file changed, 125 insertions(+), 94 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/19745/4
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/1/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/1/www/query_timeline.tmpl@450
PS1, Line 450: if (node.is_sender && node.parent_node.rendering.rownum != rownum - 1) {
> Please fix a minor bug that was introduced here in the previously commit. S
actually just the variable name needs to be updated to rownum_l.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 1
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Apr 2023 23:39:13 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Reviewed-on: http://gerrit.cloudera.org:8080/19745
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M www/query_timeline.tmpl
1 file changed, 132 insertions(+), 101 deletions(-)
Approvals:
Impala Public Jenkins: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 10
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@495
PS4, Line 495: x + px_per_tic - decimals * 6
> Posted separate PR for calculation issue https://gerrit.cloudera.org/#/c/19
That PR looks good. Thanks
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 16:20:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Hello Kurt Deschler, Wenzhe Zhou, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19745
to look at the new patch set (#7).
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
---
M www/query_timeline.tmpl
1 file changed, 132 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/19745/7
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 7
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 1:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19745/1/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/1/www/query_timeline.tmpl@351
PS1, Line 351: fragment_diagram.setAttribute("height",rownum * height + "px");
Spaces after commas on next 4 lines
http://gerrit.cloudera.org:8080/#/c/19745/1/www/query_timeline.tmpl@450
PS1, Line 450: if (node.is_sender && node.parent_node.rendering.rownum != rownum - 1) {
Please fix a minor bug that was introduced here in the previously commit. Since rownum is not maintained in this loop, there are cases that don't draw the short path to parent when plan ordering is checked.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 1
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Mon, 24 Apr 2023 23:36:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(11 comments)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@97
PS3, Line 97: return rect;
nit: indent two spaces
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@100
PS3, Line 100: fill_color
nit: add space after comma
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@111
PS3, Line 111: !=
nit: add space around !=
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@157
PS3, Line 157: if (Math.abs(dx) > ignore_px){
nit: add space in front of {
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@158
PS3, Line 158:
nit: indent two spaces
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@337
PS3, Line 337: =
nit: add space around "=" in this line and following two lines.
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@363
PS3, Line 363: ,
nit: add space after ,
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@428
PS3, Line 428: );
nit: don't split as separate line
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@444
PS3, Line 444: );
nit don't split as separate line
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@494
PS3, Line 494: x+1, y+1
nit: add spaces around +
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@497
PS3, Line 497: *
nit: add space around *
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 16:34:42 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12918/ : 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/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 7
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 17:09:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 9
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 16:15:09 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
---
M www/query_timeline.tmpl
1 file changed, 132 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/19745/6
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 6
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
(15 comments)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@39
PS3, Line 39: <input type="checkbox" id="plan_order" onClick="renderTiming()"/>
> No, it seems I missed it during the refactor. I will fix it now.
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@97
PS3, Line 97: return rect;
> nit: indent two spaces
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@100
PS3, Line 100: fill_colo
> nit: add space after comma
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@111
PS3, Line 111: !
> nit: add space around !=
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@148
PS3, Line 148: svg.appendChild(get_svg_rect(phases[color_idx].color, x + 1, y + 1,
> The -2 is deliberate to ensure that there is always a black line or outline
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@157
PS3, Line 157: var ignore_px = 2; // Don't print tiny skews
> nit: add space in front of {
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@158
PS3, Line 158: if (
> nit: indent two spaces
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@156
PS3, Line 156: var dx = (endts - ts) * px_per_ns;
: var ignore_px = 2; // Don't print tiny skews
: if (Math.abs(dx) > ignore_px) {
: svg.appendChild(get_svg_line("#505050", last_end - dx, y , last_end - dx,
:
> I will incrporate the same changes here.
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@330
PS3, Line 330: function renderTiming(ignored_arg) {
> spaces around =
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@337
PS3, Line 337:
> nit: add space around "=" in this line and following two lines.
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@363
PS3, Line 363:
> nit: add space after ,
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@428
PS3, Line 428:
> nit: don't split as separate line
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@444
PS3, Line 444:
> nit don't split as separate line
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@494
PS3, Line 494: r_tic).t
> nit: add spaces around +
Done
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@497
PS3, Line 497:
> nit: add space around *
Done
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 May 2023 09:47:03 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12801/ : 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/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 1
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 14 Apr 2023 14:27:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@148
PS3, Line 148: svg.appendChild(get_svg_rect(phases[color_idx].color, x + 1, y + 1,
> Missing width>2 check
Sorry. I will add the cases now.
Would it be better to remove the -2,
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 15:09:23 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@148
PS3, Line 148: svg.appendChild(get_svg_rect(phases[color_idx].color, x + 1, y + 1,
> Sorry. I will add the cases now.
The -2 is deliberate to ensure that there is always a black line or outline and only fill it when the width is big enough.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 22:23:20 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@342
PS4, Line 342: var height = 15;
> rename to header_footer_height
Actually row_height is more suitable as this is used more broadly
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 May 2023 15:45:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@495
PS4, Line 495: x + px_per_tic - decimals * 6
> Timestamps should be on the right so we know the end time but they should n
Posted separate PR for calculation issue https://gerrit.cloudera.org/#/c/19834/
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 14:34:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 8:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12933/ : 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/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 8
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 16:34:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@39
PS3, Line 39: <input type="checkbox" id="plan_order" onClick="refresh()"/>
Did you mean to change this to refresh() ?
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@148
PS3, Line 148: svg.appendChild(get_svg_rect(phases[color_idx].color, x + 1, y + 1,
Missing width>2 check
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@330
PS3, Line 330: var plan_order=document.getElementById('plan_order').checked;
spaces around =
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 14:55:19 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
---
M www/query_timeline.tmpl
1 file changed, 130 insertions(+), 97 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/19745/3
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@495
PS4, Line 495: x + px_per_tic - decimals * 6
> Why is "0.00" shown on the right side of the first box? "0.00" as starting
Timestamps should be on the right so we know the end time but they should not start at 0. This isn't a new issue. I will enter a separate ticket and post a fix then we can rebase this patch on top for consistency.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 14:08:59 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 7:
Since this patch has conflict with https://gerrit.cloudera.org/#/c/19834/, I will give +2 after https://gerrit.cloudera.org/#/c/19834/ is merged and this patch is rebased.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 7
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 18:06:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 6:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@100
PS4, Line 100:
> nit: add space around =
Done
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@342
PS4, Line 342: var row_height = 15;
> Actually row_height is more suitable as this is used more broadly
Done
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 6
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 16:44:41 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12906/ : 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/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 4
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 May 2023 10:07:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/4/www/query_timeline.tmpl@342
PS4, Line 342: var display_height = window.innerHeight - timing_diagram.offsetTop - 70;
rename to header_footer_height
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 02 May 2023 14:48:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 9:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/9284/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 9
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 16:15:10 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@39
PS3, Line 39: <input type="checkbox" id="plan_order" onClick="refresh()"/>
> Did you mean to change this to refresh() ?
No, it seems I missed it during the refactor. I will fix it now.
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@148
PS3, Line 148: svg.appendChild(get_svg_rect(phases[color_idx].color, x + 1, y + 1,
> Missing width>2 check
This was intentionally removed. Would it be better to add it back in the case of SVG as well, to increase the efficiency...
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 15:00:51 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl
File www/query_timeline.tmpl:
http://gerrit.cloudera.org:8080/#/c/19745/3/www/query_timeline.tmpl@156
PS3, Line 156: var ignore_px = 2; // Don't print tiny skews
: if (Math.abs(dx) > ignore_px){
: svg.appendChild(get_svg_line("#505050", last_end - dx, y , last_end - dx,
: y + bar_height, false));
: }
I will incrporate the same changes here.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 3
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Apr 2023 15:05:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/12917/ : 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/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 6
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 16:51:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 7: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 7
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 16:58:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 7:
Sure. That would be fine.
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 7
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 03 May 2023 19:34:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
Patch Set 9: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 9
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 04 May 2023 21:31:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-11970: Shifting the timeline display to SVG
Posted by "Surya Hebbar (Code Review)" <ge...@cloudera.org>.
Surya Hebbar has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/19745 )
Change subject: IMPALA-11970: Shifting the timeline display to SVG
......................................................................
IMPALA-11970: Shifting the timeline display to SVG
The usage of SVG allows for better scaling and
further manipulation of elements through the DOM,
facilitating text selection and parsing of such
elements from the diagram.
'text-anchor' and 'dominant-baseline' properties of
the SVG text element have been used to preserve the
same canvas coordinates, while shifting the center.
The maximum width behaviour of canvas text has been
implemented using 'textLength' and 'lengthAdjust'.
Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
---
M www/query_timeline.tmpl
1 file changed, 132 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/45/19745/8
--
To view, visit http://gerrit.cloudera.org:8080/19745
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I083c2ec12e1743b89092fc23281ee576d66fa81b
Gerrit-Change-Number: 19745
Gerrit-PatchSet: 8
Gerrit-Owner: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Surya Hebbar <sh...@cloudera.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>