Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Printer for the plan, ie: EXPLAIN #2075

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

mamcx
Copy link
Contributor

@mamcx mamcx commented Dec 19, 2024

Description of Changes

Incipient support for EXPLAIN with close plan output as in Postgres (but not yet conformant).

It has the extra capability of showing extra metadata about the schema & indexes, useful for testing & debugging.

Example:

-- Without metadata
Nested Loop
  -> Index Scan using Index id 0: (employee) on m
    -> Index Cond: (m.employee = U64(1))
  -> Seq Scan on p
  Output: id, name
Planning Time: 541.5µs

-- With metadata
Query: SELECT m.* FROM m CROSS JOIN p WHERE m.employee = 1
Nested Loop
  -> Index Scan using Index id 0: (employee) on m:1
    -> Index Cond: (m.employee = U64(1))
  -> Seq Scan on p:2
  Output: id, name
Planning Time: 527.666µs
-------
Schema:

Label m: 1
  Columns: employee, manager
  Indexes: Unique(m.employee)
Label p: 2
  Columns: id, name
  Indexes: Unique(p.id)

Closes #2058.

API and ABI breaking changes

None

Expected complexity level and risk

1

Testing

  • Adding testing for both optimized or not variations of the plan
  • WILL NEED more test but is likely that we need to do another pr

@mamcx mamcx added no runtime change This change does not affect the final binaries release-1.0 labels Dec 19, 2024
@mamcx mamcx self-assigned this Dec 19, 2024
@mamcx mamcx force-pushed the mamcx/planner-printer branch from be18b08 to 81adebc Compare December 24, 2024 15:58
@mamcx mamcx force-pushed the mamcx/planner-printer branch from 1ca3567 to 75e1cbd Compare January 6, 2025 17:24
@mamcx mamcx changed the base branch from master to joshua/query-engine-integration January 6, 2025 17:26
@mamcx mamcx changed the base branch from joshua/query-engine-integration to master January 6, 2025 17:27
@mamcx mamcx force-pushed the mamcx/planner-printer branch 5 times, most recently from 305e0bc to 33c8978 Compare January 14, 2025 15:21
@mamcx mamcx force-pushed the mamcx/planner-printer branch from 33c8978 to f8700bc Compare January 15, 2025 16:27
@mamcx mamcx force-pushed the mamcx/planner-printer branch 2 times, most recently from 1aeeae9 to a9569f6 Compare January 27, 2025 16:53
@mamcx mamcx marked this pull request as ready for review January 27, 2025 17:05
@mamcx mamcx force-pushed the mamcx/planner-printer branch from a9569f6 to c592668 Compare January 29, 2025 18:01
@mamcx mamcx force-pushed the mamcx/planner-printer branch from c592668 to 2f53089 Compare February 4, 2025 18:55
@mamcx mamcx force-pushed the mamcx/planner-printer branch from 2f53089 to 692ebe2 Compare February 4, 2025 19:02
@mamcx mamcx marked this pull request as draft February 4, 2025 19:07
@mamcx mamcx force-pushed the mamcx/planner-printer branch 2 times, most recently from 4cc69d6 to d954c44 Compare February 5, 2025 20:09
@mamcx mamcx force-pushed the mamcx/planner-printer branch 2 times, most recently from 419de9e to 3681c2c Compare February 6, 2025 16:57
@mamcx mamcx marked this pull request as ready for review February 6, 2025 18:14
@bfops
Copy link
Collaborator

bfops commented Feb 14, 2025

Is this achieving the same thing as #2065 ?

@mamcx
Copy link
Contributor Author

mamcx commented Feb 14, 2025

Is this achieving the same thing as #2065 ?

No, this is for print the sql aka EXPLAIN SELECT... that other is for show the steps of migrations

@bfops bfops added release-any To be landed in any release window and removed no runtime change This change does not affect the final binaries labels Feb 18, 2025
mamcx added 3 commits March 5, 2025 10:03
* Improve output of plan
* Fix print of alias and the output of joins
* Allow the test utils of the plan to be used elsewhere
* Use the arrow only for nodes, make timing optional
* Apply new clippy hints and remove old test logic
@mamcx mamcx force-pushed the mamcx/planner-printer branch from 45e5e10 to 8ede66f Compare March 5, 2025 17:45
@mamcx mamcx requested a review from joshua-spacetime March 5, 2025 17:50
@bfops bfops removed their request for review March 6, 2025 19:22
@bfops
Copy link
Collaborator

bfops commented Mar 6, 2025

I'm removing myself as a reviewer, because there is no longer any review for the files that I am a CODEOWNER of.

@@ -364,6 +364,7 @@ pub(crate) mod tests {
let result = run_for_testing(&db, sql)?;
assert_eq!(result, vec![product![5u64]], "Inventory");

// TODO: This should return product![2u64]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// TODO: This should return product![2u64]

Aggregates are applied before LIMIT.

Comment on lines +1394 to +1405
Index Join: Rhs on b
-> Index Join: Rhs on q
-> Index Join: Rhs on p
-> Index Scan using Index id 0 on u
Index Cond: (u.identity = U64(5))
Inner Unique: true
Join Cond: (u.entity_id = p.entity_id)
Inner Unique: false
Join Cond: (p.chunk = q.chunk)
Inner Unique: true
Join Cond: (q.entity_id = b.entity_id)
Output: b.entity_id, b.misc"#]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think I would like this to look something like:

Index Join
    Output: (b: RowId (table: b))
    Index: <index name> (table: b, unique: true)
    Index Cond: q.entity_id
    -> Index Join
        Output: (q: RowId (table: b))
        Index: <index name> (table: l, unique: false)
        Index Cond: p.chunk
        -> Index Join
            Output: (p: RowId (table: l))
            Index: <index name> (table: l, unique: true)
            Index Cond: u.entity_id
            -> Index Scan
                Output: (u: RowId (table: u))
                Index Cond: (identity = U64(5))

or

Index Join using <index name> on b
    Output: (b: RowId (table: b))
    Index Cond: q.entity_id
    -> Index Join using <index name> on l
        Output: (q: RowId (table: l))
        Index Cond: p.chunk
        -> Index Join using <index name> on l
            Output: (p: RowId (table: l))
            Index Cond: u.entity_id
            -> Index Scan using <index name> on u
                Output: (u: RowId (table: u))
                Index Cond: (identity = U64(5))

I.e., I want to know the index id or the index name that is being probed in an index join. I also want to know if an operator is returning row pointers vs product values.

Comment on lines +1495 to +1513
Hash Join
-> Hash Join
-> Hash Join
-> Hash Join
-> Seq Scan on m
-> Seq Scan on n
Inner Unique: false
Join Cond: (m.manager = n.manager)
-> Seq Scan on u
Inner Unique: false
Join Cond: (n.employee = u.employee)
-> Seq Scan on v
Inner Unique: false
Join Cond: (u.project = v.project)
-> Seq Scan on p
Inner Unique: false
Join Cond: (p.id = v.project)
Filter: (m.employee = U64(5) AND v.employee = U64(5))
Output: p.id, p.name"#]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this is correct. This looks like the plan before it is optimized. Most of these hash joins should be index joins and the sequential scans should be index scans or index probes.

Comment on lines +1863 to +1865
Limit: 5
-> Seq Scan on t
Output: t.x, t.y"#]],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
Limit: 5
-> Seq Scan on t
Output: t.x, t.y"#]],
Limit: 5
Output: (t: RowId (table: t))
-> Seq Scan on t"#]],

I know this is a deviation from pg, but I find it much easier to read if the output comes before the input. In particular for left deep joins.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release-any To be landed in any release window
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Query plan printer
4 participants