dalibo / pev2

Postgres Explain Visualizer 2
https://explain.dalibo.com
PostgreSQL License
2.58k stars 125 forks source link

#626 The diagram view includes 'Parallel' for parallel-aware nodes #638

Closed dkarneman closed 6 months ago

dkarneman commented 6 months ago

Per #626, the Diagram doesn't include Parallel for parallel-aware nodes (Seq Scan, Append, Hash, etc). The 'Parallel' modifier is stripped during parsing and then added back in the graph view.

This PR re-uses the nodeName function to make the two match.

Render of src/services/__tests__/12-line-wrapped-plans/05-plan Screenshot_2024-03-21_at_11_31_22

I considered removing the line that strips out 'Parallel'. But that would alter the NODE_TYPE and it's not clear to me whether nodes of the same type are handled separately (i.e. do all 'Seq Scans' need to be treated alike, regardless of whether they're parallel?)

Test Plan

Existing tests pass, and lints cleanly (except for one lint that is not modified by this PR.)

I'd love to write a test for this change, but I don't see existing examples that test the view, just the parser. Please point me in the right direction if there's a good way to test the view.

pgiraud commented 6 months ago

I considered removing the line that strips out 'Parallel'. But that would alter the NODE_TYPE and it's not clear to me whether nodes of the same type are handled separately (i.e. do all 'Seq Scans' need to be treated alike, regardless of whether they're parallel?)

The reason why the Parallel is removed from NODE TYPE is to match the JSON format. When parsing a plan in the text format, we try to convert it to a JS object that ressemble the explain JSON format as much as possible. See https://github.com/dalibo/pev2/blob/master/example/src/samples.ts#L5790-L5792 for an example of a plan in JSON format with parallel nodes.

Existing tests pass, and lints cleanly (except for one lint that is not modified by this PR.)

Thanks for the report. I'll fix that.

I'd love to write a test for this change, but I don't see existing examples that test the view, just the parser. Please point me in the right direction if there's a good way to test the view.

There's unfortunately no tests for the rendering. I really need to add some.