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

More cursor-related details in the plan output #7441

Merged
merged 2 commits into from
Jan 10, 2023

Conversation

dyemanov
Copy link
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

…s, show cursor names/options, report line numbers
@dyemanov dyemanov self-assigned this Jan 2, 2023

// SubQuery class (simplified forward-only cursor)

class SubQuery : public Select
Copy link
Member

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.

@@ -130,7 +179,7 @@ void Cursor::close(thread_db* tdbb) const

bool Cursor::fetchNext(thread_db* tdbb) const
{
if (m_scrollable)
if (m_rse->flags & RseNode::FLAG_SCROLLABLE)
Copy link
Member

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
Member

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

sim1984 commented Jan 3, 2023

It does not interfere.

@dyemanov
Copy link
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
Member

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.

@dyemanov
Copy link
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
Member

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
Member

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.

@pavel-zotov
Copy link

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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants