Skip to content

More cursor-related details in the plan output#7441

Merged
dyemanov merged 2 commits intomasterfrom
v5-pretty-explained-plan
Jan 10, 2023
Merged

More cursor-related details in the plan output#7441
dyemanov merged 2 commits intomasterfrom
v5-pretty-explained-plan

Conversation

@dyemanov
Copy link
Copy Markdown
Member

@dyemanov dyemanov commented Jan 2, 2023

  1. Main queries are still reported as "select expression", while its sub-queries are reported as "sub-query", and declared cursors are reported as "cursor ".
  2. Invariant sub-queries and scrollable cursors are reported as such (see also Fix invariants optimization involving views (#7388) #7390).
  3. If the plan is requested for EXECUTE BLOCK or some cached PSQL object (procedure/function/trigger), every select expression inside that PSQL module is prefixed with its position (line/column numbers).

Examples:

-- line 23, column 2
PLAN (DISTRICT INDEX (DISTRICT_PK))
-- line 28, column 2
PLAN JOIN (CUSTOMER INDEX (CUSTOMER_PK), WAREHOUSE INDEX(WAREHOUSE_PK))
Select Expression (line 23, column 2)
    -> Singularity Check
        -> Filter
            -> Table "DISTRICT" Access By ID
                -> Bitmap
                    -> Index "DISTRICT_PK" Unique Scan
Select Expression (line 28, column 2)
    -> Singularity Check
        -> Nested Loop Join (inner)
            -> Filter
                -> Table "CUSTOMER" Access By ID
                    -> Bitmap
                        -> Index "CUSTOMER_PK" Unique Scan
            -> Filter
                -> Table "WAREHOUSE" Access By ID
                    -> Bitmap
                        -> Index "WAREHOUSE_PK" Unique Scan

@dyemanov
Prettier plan output: distinguish between main queries and sub-querie…
36d011f 
…s, show cursor names/options, report line numbers
@dyemanov dyemanov self-assigned this Jan 2, 2023
asfernandes
asfernandes reviewed Jan 3, 2023
View reviewed changes
Comment thread src/jrd/recsrc/Cursor.h Outdated

// SubQuery class (simplified forward-only cursor)

class SubQuery : public Select
Copy link
Copy Markdown
Member

@asfernandes asfernandes Jan 3, 2023

Choose a reason for hiding this comment

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

Looks like SubQuery and Cursor may be declared as final classes.

Comment thread src/jrd/recsrc/Cursor.cpp Outdated
bool Cursor::fetchNext(thread_db* tdbb) const
{
if (m_scrollable)
if (m_rse->flags & RseNode::FLAG_SCROLLABLE)
Copy link
Copy Markdown
Member

@asfernandes asfernandes Jan 3, 2023

Choose a reason for hiding this comment

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

An inline isScrollable() method would make code more clear.

@asfernandes
Copy link
Copy Markdown
Member

asfernandes commented Jan 3, 2023

Is this going to beta 1?

I already started to add same information to the profiler, with means another table for cursor data (it was under a TODO, but now that data should be available) and would like to avoid add migrations after beta 1.

@sim1984
Copy link
Copy Markdown
Contributor

sim1984 commented Jan 3, 2023

It does not interfere.

@dyemanov
Copy link
Copy Markdown
Member Author

dyemanov commented Jan 3, 2023

This patch is not critical for Beta (which is late enough), so I planned to merge it later.
IMHO, PR #7397 is more important for Beta, but nobody reacted on it yet :-(

@asfernandes
Copy link
Copy Markdown
Member

asfernandes commented Jan 4, 2023

This comment became not true as now the name of FOR ... AS CURSOR <name> became visible.
This name is not being displayed in the plan.

// ASF: We cannot define the name of the cursor here, but this is not a problem,
// as implicit cursors are always positioned in a valid record, and the name is
// only used to raise isc_cursor_not_positioned.

@asfernandes asfernandes mentioned this pull request Jan 4, 2023
Merged
@dyemanov
Copy link
Copy Markdown
Member Author

dyemanov commented Jan 4, 2023

I print cursor names only for DECLARE CURSOR. I didn't intend to distinguish between FOR SELECT and FOR SELECT AS CURSOR C. Perhaps it would be handy, but I don't see how this name can be retrieved at the JRD level.

@asfernandes
Copy link
Copy Markdown
Member

asfernandes commented Jan 4, 2023

Perhaps it would be handy, but I don't see how this name can be retrieved at the JRD level.

We can't do it with fb_dbg_map_curname but we can do using something similar to fb_dbg_map_src2blr where we can associate position of blr_for with the name.

I'm implementing it for #7442.

@asfernandes
Copy link
Copy Markdown
Member

asfernandes commented Jan 5, 2023

We can't do it with fb_dbg_map_curname but we can do using something similar to fb_dbg_map_src2blr where we can associate position of blr_for with the name.

I'm implementing it for #7442.

I committed it (74a18d9).

It's much more related to this branch, so we can cherry-pick it here without conflicts if you want.

@dyemanov dyemanov merged commit a6ce0ec into master Jan 10, 2023
@dyemanov dyemanov deleted the v5-pretty-explained-plan branch January 10, 2023 17:58
@pavel-zotov
Copy link
Copy Markdown

pavel-zotov commented Dec 18, 2023

QA note.
Test not needed: issues are covered by (at least) bugs/gh_7466_plans_tracking_test.py and bugs/gh_7853_test.py

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@asfernandes asfernandes asfernandes left review comments

Assignees

@dyemanov dyemanov

Labels

component: engine fix-version: 5.0 Beta 1 qa: covered by another tests type: improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@dyemanov @asfernandes @sim1984 @pavel-zotov