Обсуждение: pgsql: Adjust things so that the query_string of a cached plan and the

Поиск
Список
Период
Сортировка

pgsql: Adjust things so that the query_string of a cached plan and the

От
tgl@postgresql.org (Tom Lane)
Дата:
Log Message:
-----------
Adjust things so that the query_string of a cached plan and the sourceText of
a portal are never NULL, but reliably provide the source text of the query.
It turns out that there was only one place that was really taking a short-cut,
which was the 'EXECUTE' utility statement.  That doesn't seem like a
sufficiently critical performance hotspot to justify not offering a guarantee
of validity of the portal source text.  Fix it to copy the source text over
from the cached plan.  Add Asserts in the places that set up cached plans and
portals to reject null source strings, and simplify a bunch of places that
formerly needed to guard against nulls.

There may be a few places that cons up statements for execution without
having any source text at all; I found one such in ConvertTriggerToFK().
It seems sufficient to inject a phony source string in such a case,
for instance
        ProcessUtility((Node *) atstmt,
                       "(generated ALTER TABLE ADD FOREIGN KEY command)",
                       NULL, false, None_Receiver, NULL);

We should take a second look at the usage of debug_query_string,
particularly the recently added current_query() SQL function.

ITAGAKI Takahiro and Tom Lane

Modified Files:
--------------
    pgsql/src/backend/commands:
        portalcmds.c (r1.74 -> r1.75)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/portalcmds.c?r1=1.74&r2=1.75)
        prepare.c (r1.87 -> r1.88)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/prepare.c?r1=1.87&r2=1.88)
        trigger.c (r1.235 -> r1.236)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/trigger.c?r1=1.235&r2=1.236)
    pgsql/src/backend/executor:
        spi.c (r1.196 -> r1.197)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/executor/spi.c?r1=1.196&r2=1.197)
    pgsql/src/backend/parser:
        analyze.c (r1.372 -> r1.373)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/parser/analyze.c?r1=1.372&r2=1.373)
    pgsql/src/backend/tcop:
        postgres.c (r1.553 -> r1.554)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/postgres.c?r1=1.553&r2=1.554)
        utility.c (r1.294 -> r1.295)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/utility.c?r1=1.294&r2=1.295)
    pgsql/src/backend/utils/cache:
        plancache.c (r1.18 -> r1.19)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/cache/plancache.c?r1=1.18&r2=1.19)
    pgsql/src/backend/utils/mmgr:
        portalmem.c (r1.110 -> r1.111)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/mmgr/portalmem.c?r1=1.110&r2=1.111)
    pgsql/src/include/utils:
        plancache.h (r1.11 -> r1.12)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/plancache.h?r1=1.11&r2=1.12)
        portal.h (r1.78 -> r1.79)
        (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/portal.h?r1=1.78&r2=1.79)

Re: pgsql: Adjust things so that the query_string of a cached plan and the

От
Bruce Momjian
Дата:
This commit mentions checking 'debug_query_string' again;  exactly what
needs checking?

---------------------------------------------------------------------------

Tom Lane wrote:
> Log Message:
> -----------
> Adjust things so that the query_string of a cached plan and the sourceText of
> a portal are never NULL, but reliably provide the source text of the query.
> It turns out that there was only one place that was really taking a short-cut,
> which was the 'EXECUTE' utility statement.  That doesn't seem like a
> sufficiently critical performance hotspot to justify not offering a guarantee
> of validity of the portal source text.  Fix it to copy the source text over
> from the cached plan.  Add Asserts in the places that set up cached plans and
> portals to reject null source strings, and simplify a bunch of places that
> formerly needed to guard against nulls.
>
> There may be a few places that cons up statements for execution without
> having any source text at all; I found one such in ConvertTriggerToFK().
> It seems sufficient to inject a phony source string in such a case,
> for instance
>         ProcessUtility((Node *) atstmt,
>                        "(generated ALTER TABLE ADD FOREIGN KEY command)",
>                        NULL, false, None_Receiver, NULL);
>
> We should take a second look at the usage of debug_query_string,
> particularly the recently added current_query() SQL function.
>
> ITAGAKI Takahiro and Tom Lane
>
> Modified Files:
> --------------
>     pgsql/src/backend/commands:
>         portalcmds.c (r1.74 -> r1.75)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/portalcmds.c?r1=1.74&r2=1.75)
>         prepare.c (r1.87 -> r1.88)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/prepare.c?r1=1.87&r2=1.88)
>         trigger.c (r1.235 -> r1.236)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/trigger.c?r1=1.235&r2=1.236)
>     pgsql/src/backend/executor:
>         spi.c (r1.196 -> r1.197)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/executor/spi.c?r1=1.196&r2=1.197)
>     pgsql/src/backend/parser:
>         analyze.c (r1.372 -> r1.373)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/parser/analyze.c?r1=1.372&r2=1.373)
>     pgsql/src/backend/tcop:
>         postgres.c (r1.553 -> r1.554)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/postgres.c?r1=1.553&r2=1.554)
>         utility.c (r1.294 -> r1.295)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/tcop/utility.c?r1=1.294&r2=1.295)
>     pgsql/src/backend/utils/cache:
>         plancache.c (r1.18 -> r1.19)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/cache/plancache.c?r1=1.18&r2=1.19)
>     pgsql/src/backend/utils/mmgr:
>         portalmem.c (r1.110 -> r1.111)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/utils/mmgr/portalmem.c?r1=1.110&r2=1.111)
>     pgsql/src/include/utils:
>         plancache.h (r1.11 -> r1.12)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/plancache.h?r1=1.11&r2=1.12)
>         portal.h (r1.78 -> r1.79)
>         (http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/include/utils/portal.h?r1=1.78&r2=1.79)
>
> --
> Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-committers

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: pgsql: Adjust things so that the query_string of a cached plan and the

От
Tom Lane
Дата:
Bruce Momjian <bruce@momjian.us> writes:
> This commit mentions checking 'debug_query_string' again;  exactly what
> needs checking?

Mainly, whether we should still be using it at all anywhere.

            regards, tom lane