Skip to content

[Native] Fix PartitionedOutputNode plan node id#21742

Merged
zacw7 merged 1 commit intoprestodb:masterfrom
kewang1024:Fix-output-operator-planNodeId
Jan 29, 2024
Merged

[Native] Fix PartitionedOutputNode plan node id#21742
zacw7 merged 1 commit intoprestodb:masterfrom
kewang1024:Fix-output-operator-planNodeId

Conversation

@kewang1024
Copy link
Collaborator

@kewang1024 kewang1024 commented Jan 20, 2024

Presto doesn't have PartitionedOutputNode and assigns its source node's plan
node id to PartitionedOutputOperator.
so Presto UI is expecting PartitionedOutputOperator having same node id as its
source operator.
However, Velox has PartitionedOutputNode whose plan node is is set to "root".

To comply with original assumption, fix partitionedOutputNode plan node id to be
root.<source plan node ID> and parse them back to source plan node ID in task
stats reporting.

Fixes #21741

@kewang1024
Copy link
Collaborator Author

kewang1024 commented Jan 20, 2024

Steps to Reproduce

Set up both environments locally:

  • Presto HiveQueryRunner [1 worker]
  • Prestissimo environment (HiveExternalWorkerQueryRunner) [1 worker]

Ran query in both environments:
insert into orders_ke select orderkey, custkey, orderstatus, totalprice, '2011-01-01', '1', 'clerk', 2, 'comment' from orders;

JAVA VS CPP
image

After the change

Ran query in CPP environment:
image

@kewang1024 kewang1024 force-pushed the Fix-output-operator-planNodeId branch from db2730b to 3b22d2f Compare January 29, 2024 01:21
@kewang1024 kewang1024 changed the title [Native] Output operator should use the same plan node id as its source [Native] Fix PartitionedOutputNode plan node id Jan 29, 2024
@kewang1024 kewang1024 force-pushed the Fix-output-operator-planNodeId branch from 3b22d2f to 3ae8a30 Compare January 29, 2024 01:35
@kewang1024 kewang1024 marked this pull request as ready for review January 29, 2024 01:36
@kewang1024 kewang1024 requested a review from a team as a code owner January 29, 2024 01:36
@kewang1024 kewang1024 force-pushed the Fix-output-operator-planNodeId branch from 3ae8a30 to 1aa3032 Compare January 29, 2024 01:37
Copy link
Contributor

@xiaoxmeng xiaoxmeng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kewang1024 LGTM. Thanks!

@kewang1024 kewang1024 force-pushed the Fix-output-operator-planNodeId branch 4 times, most recently from bb55a3b to a1a9bfe Compare January 29, 2024 19:20
Presto doesn't have PartitionedOutputNode and assigns its source node's plan
node id to PartitionedOutputOperator.
so Presto UI is expecting PartitionedOutputOperator having same node id as its
source operator.
However, Velox has PartitionedOutputNode whose plan node is is set to "root".

To comply with original assumption, fix partitionedOutputNode plan node id to be
"root.<source plan node ID>" and parse them back to source plan node ID in task
stats reporting.

Issue: prestodb#21741
@kewang1024 kewang1024 force-pushed the Fix-output-operator-planNodeId branch from a1a9bfe to 7e20828 Compare January 29, 2024 19:52
@zacw7 zacw7 merged commit e6b024f into prestodb:master Jan 29, 2024
@wanglinsong wanglinsong mentioned this pull request Feb 12, 2024
64 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[native] Output operators are missing in Stage Summary Page

3 participants