pgsql: Avoid unnecessary plancache revalidation of utility statements.

Поиск
Список
Период
Сортировка
От Tom Lane
Тема pgsql: Avoid unnecessary plancache revalidation of utility statements.
Дата
Msg-id E1qZCnj-000gGp-Pp@gemulon.postgresql.org
обсуждение исходный текст
Список pgsql-committers
Avoid unnecessary plancache revalidation of utility statements.

Revalidation of a plancache entry (after a cache invalidation event)
requires acquiring a snapshot.  Normally that is harmless, but not
if the cached statement is one that needs to run without acquiring a
snapshot.  We were already aware of that for TransactionStmts,
but for some reason hadn't extrapolated to the other statements that
PlannedStmtRequiresSnapshot() knows mustn't set a snapshot.  This can
lead to unexpected failures of commands such as SET TRANSACTION
ISOLATION LEVEL.  We can fix it in the same way, by excluding those
command types from revalidation.

However, we can do even better than that: there is no need to
revalidate for any statement type for which parse analysis, rewrite,
and plan steps do nothing interesting, which is nearly all utility
commands.  To mechanize this, invent a parser function
stmt_requires_parse_analysis() that tells whether parse analysis does
anything beyond wrapping a CMD_UTILITY Query around the raw parse
tree.  If that's what it does, then rewrite and plan will just
skip the Query, so that it is not possible for the same raw parse
tree to produce a different plan tree after cache invalidation.

stmt_requires_parse_analysis() is basically equivalent to the
existing function analyze_requires_snapshot(), except that for
obscure reasons that function omits ReturnStmt and CallStmt.
It is unclear whether those were oversights or intentional.
I have not been able to demonstrate a bug from not acquiring a
snapshot while analyzing these commands, but at best it seems mighty
fragile.  It seems safer to acquire a snapshot for parse analysis of
these commands too, which allows making stmt_requires_parse_analysis
and analyze_requires_snapshot equivalent.

In passing this fixes a second bug, which is that ResetPlanCache
would exclude ReturnStmts and CallStmts from revalidation.
That's surely *not* safe, since they contain parsable expressions.

Per bug #18059 from Pavel Kulakov.  Back-patch to all supported
branches.

Discussion: https://postgr.es/m/18059-79c692f036b25346@postgresql.org

Branch
------
REL_11_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/9c59f3862b182dffa85e7d1ea924187c22ace086

Modified Files
--------------
src/backend/parser/analyze.c                 | 51 +++++++++++++++++---
src/backend/utils/cache/plancache.c          | 70 +++++++++++-----------------
src/include/parser/analyze.h                 |  1 +
src/pl/plpgsql/src/expected/plpgsql_call.out | 17 +++++++
src/pl/plpgsql/src/sql/plpgsql_call.sql      | 18 +++++++
5 files changed, 107 insertions(+), 50 deletions(-)


В списке pgsql-committers по дате отправления:

Предыдущее
От: Tom Lane
Дата:
Сообщение: Re: pgsql: Cache by-reference missing values in a long lived context
Следующее
От: Nathan Bossart
Дата:
Сообщение: pgsql: pg_upgrade: Bump MESSAGE_WIDTH.