Skip to content

Add a null-check for stored plan text in pg_store_plans_internal(). #35

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add a null-check for stored plan text in pg_store_plans_internal(). #35

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions pg_store_plans.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1579,6 +1579,12 @@ pg_store_plans_internal(FunctionCallInfo fcinfo,
else
pstr = SHMEM_PLAN_PTR(entry);

if (pstr == NULL)
{
values[i++] = CStringGetTextDatum("<invalid plan>");
Copy link
Collaborator

@rjuju rjuju Feb 2, 2025

Choose a reason for hiding this comment

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

I think it would be better to set the value as null rather than adding another hardcoded value that users will have to explicitly check.

Also, since the code is not that complicated it would be better to avoid a gotoand just nest the instructions.

goto outofblock01;
}

switch (plan_format)
{
case PLAN_FORMAT_TEXT:
Expand Down Expand Up @@ -1612,10 +1618,13 @@ pg_store_plans_internal(FunctionCallInfo fcinfo,
pfree(mstr);

/* pstr is a pointer onto pbuffer */

Copy link
Collaborator

@rjuju rjuju Feb 2, 2025

Choose a reason for hiding this comment

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

please avoid some unrelated changes like that.

}
else
values[i++] = CStringGetTextDatum("<insufficient privilege>");

outofblock01:

/* copy counters to a local variable to keep locking time short */
{
volatile pgspEntry *e = (volatile pgspEntry *) entry;
Expand Down