mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-02 21:07:20 +02:00
Explicitly support the case that a plancache's raw_parse_tree is NULL.
This only happens if a client issues a Parse message with an empty query string, which is a bit odd; but since it is explicitly called out as legal by our FE/BE protocol spec, we'd probably better continue to allow it. Fix by adding tests everywhere that the raw_parse_tree field is passed to functions that don't or shouldn't accept NULL. Also make it clear in the relevant comments that NULL is an expected case. This reverts commitsa73c9dbab0
and2e9650cbcf
, which fixed specific crash symptoms by hacking things at what now seems to be the wrong end, ie the callee functions. Making the callees allow NULL is superficially more robust, but it's not always true that there is a defensible thing for the callee to do in such cases. The caller has more context and is better able to decide what the empty-query case ought to do. Per followup discussion of bug #11335. Back-patch to 9.2. The code before that is sufficiently different that it would require development of a separate patch, which doesn't seem worthwhile for what is believed to be an essentially cosmetic change.
This commit is contained in:
parent
5005469cb2
commit
40b85a3ab1
@ -2037,7 +2037,9 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
|
|||||||
* Parameter datatypes are driven by parserSetup hook if provided,
|
* Parameter datatypes are driven by parserSetup hook if provided,
|
||||||
* otherwise we use the fixed parameter list.
|
* otherwise we use the fixed parameter list.
|
||||||
*/
|
*/
|
||||||
if (plan->parserSetup != NULL)
|
if (parsetree == NULL)
|
||||||
|
stmt_list = NIL;
|
||||||
|
else if (plan->parserSetup != NULL)
|
||||||
{
|
{
|
||||||
Assert(plan->nargs == 0);
|
Assert(plan->nargs == 0);
|
||||||
stmt_list = pg_analyze_and_rewrite_params(parsetree,
|
stmt_list = pg_analyze_and_rewrite_params(parsetree,
|
||||||
|
@ -288,17 +288,13 @@ transformStmt(ParseState *pstate, Node *parseTree)
|
|||||||
* Returns true if a snapshot must be set before doing parse analysis
|
* Returns true if a snapshot must be set before doing parse analysis
|
||||||
* on the given raw parse tree.
|
* on the given raw parse tree.
|
||||||
*
|
*
|
||||||
* Classification here should match transformStmt(); but we also have to
|
* Classification here should match transformStmt().
|
||||||
* allow a NULL input (for Parse/Bind of an empty query string).
|
|
||||||
*/
|
*/
|
||||||
bool
|
bool
|
||||||
analyze_requires_snapshot(Node *parseTree)
|
analyze_requires_snapshot(Node *parseTree)
|
||||||
{
|
{
|
||||||
bool result;
|
bool result;
|
||||||
|
|
||||||
if (parseTree == NULL)
|
|
||||||
return false;
|
|
||||||
|
|
||||||
switch (nodeTag(parseTree))
|
switch (nodeTag(parseTree))
|
||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
|
@ -1546,7 +1546,9 @@ exec_bind_message(StringInfo input_message)
|
|||||||
* snapshot active till we're done, so that plancache.c doesn't have to
|
* snapshot active till we're done, so that plancache.c doesn't have to
|
||||||
* take new ones.
|
* take new ones.
|
||||||
*/
|
*/
|
||||||
if (numParams > 0 || analyze_requires_snapshot(psrc->raw_parse_tree))
|
if (numParams > 0 ||
|
||||||
|
(psrc->raw_parse_tree &&
|
||||||
|
analyze_requires_snapshot(psrc->raw_parse_tree)))
|
||||||
{
|
{
|
||||||
PushActiveSnapshot(GetTransactionSnapshot());
|
PushActiveSnapshot(GetTransactionSnapshot());
|
||||||
snapshot_set = true;
|
snapshot_set = true;
|
||||||
|
@ -2452,9 +2452,6 @@ GetCommandLogLevel(Node *parsetree)
|
|||||||
{
|
{
|
||||||
LogStmtLevel lev;
|
LogStmtLevel lev;
|
||||||
|
|
||||||
if (parsetree == NULL)
|
|
||||||
return LOGSTMT_ALL;
|
|
||||||
|
|
||||||
switch (nodeTag(parsetree))
|
switch (nodeTag(parsetree))
|
||||||
{
|
{
|
||||||
/* raw plannable queries */
|
/* raw plannable queries */
|
||||||
@ -2558,7 +2555,7 @@ GetCommandLogLevel(Node *parsetree)
|
|||||||
|
|
||||||
/* Look through an EXECUTE to the referenced stmt */
|
/* Look through an EXECUTE to the referenced stmt */
|
||||||
ps = FetchPreparedStatement(stmt->name, false);
|
ps = FetchPreparedStatement(stmt->name, false);
|
||||||
if (ps)
|
if (ps && ps->plansource->raw_parse_tree)
|
||||||
lev = GetCommandLogLevel(ps->plansource->raw_parse_tree);
|
lev = GetCommandLogLevel(ps->plansource->raw_parse_tree);
|
||||||
else
|
else
|
||||||
lev = LOGSTMT_ALL;
|
lev = LOGSTMT_ALL;
|
||||||
|
9
src/backend/utils/cache/plancache.c
vendored
9
src/backend/utils/cache/plancache.c
vendored
@ -139,7 +139,7 @@ InitPlanCache(void)
|
|||||||
* Once constructed, the cached plan can be made longer-lived, if needed,
|
* Once constructed, the cached plan can be made longer-lived, if needed,
|
||||||
* by calling SaveCachedPlan.
|
* by calling SaveCachedPlan.
|
||||||
*
|
*
|
||||||
* raw_parse_tree: output of raw_parser()
|
* raw_parse_tree: output of raw_parser(), or NULL if empty query
|
||||||
* query_string: original query text
|
* query_string: original query text
|
||||||
* commandTag: compile-time-constant tag for query, or NULL if empty query
|
* commandTag: compile-time-constant tag for query, or NULL if empty query
|
||||||
*/
|
*/
|
||||||
@ -221,7 +221,7 @@ CreateCachedPlan(Node *raw_parse_tree,
|
|||||||
* invalidation, so plan use must be completed in the current transaction,
|
* invalidation, so plan use must be completed in the current transaction,
|
||||||
* and DDL that might invalidate the querytree_list must be avoided as well.
|
* and DDL that might invalidate the querytree_list must be avoided as well.
|
||||||
*
|
*
|
||||||
* raw_parse_tree: output of raw_parser()
|
* raw_parse_tree: output of raw_parser(), or NULL if empty query
|
||||||
* query_string: original query text
|
* query_string: original query text
|
||||||
* commandTag: compile-time-constant tag for query, or NULL if empty query
|
* commandTag: compile-time-constant tag for query, or NULL if empty query
|
||||||
*/
|
*/
|
||||||
@ -659,7 +659,9 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
|
|||||||
* the cache.
|
* the cache.
|
||||||
*/
|
*/
|
||||||
rawtree = copyObject(plansource->raw_parse_tree);
|
rawtree = copyObject(plansource->raw_parse_tree);
|
||||||
if (plansource->parserSetup != NULL)
|
if (rawtree == NULL)
|
||||||
|
tlist = NIL;
|
||||||
|
else if (plansource->parserSetup != NULL)
|
||||||
tlist = pg_analyze_and_rewrite_params(rawtree,
|
tlist = pg_analyze_and_rewrite_params(rawtree,
|
||||||
plansource->query_string,
|
plansource->query_string,
|
||||||
plansource->parserSetup,
|
plansource->parserSetup,
|
||||||
@ -887,6 +889,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
|
|||||||
*/
|
*/
|
||||||
snapshot_set = false;
|
snapshot_set = false;
|
||||||
if (!ActiveSnapshotSet() &&
|
if (!ActiveSnapshotSet() &&
|
||||||
|
plansource->raw_parse_tree &&
|
||||||
analyze_requires_snapshot(plansource->raw_parse_tree))
|
analyze_requires_snapshot(plansource->raw_parse_tree))
|
||||||
{
|
{
|
||||||
PushActiveSnapshot(GetTransactionSnapshot());
|
PushActiveSnapshot(GetTransactionSnapshot());
|
||||||
|
@ -76,7 +76,7 @@
|
|||||||
typedef struct CachedPlanSource
|
typedef struct CachedPlanSource
|
||||||
{
|
{
|
||||||
int magic; /* should equal CACHEDPLANSOURCE_MAGIC */
|
int magic; /* should equal CACHEDPLANSOURCE_MAGIC */
|
||||||
Node *raw_parse_tree; /* output of raw_parser() */
|
Node *raw_parse_tree; /* output of raw_parser(), or NULL */
|
||||||
const char *query_string; /* source text of query */
|
const char *query_string; /* source text of query */
|
||||||
const char *commandTag; /* command tag (a constant!), or NULL */
|
const char *commandTag; /* command tag (a constant!), or NULL */
|
||||||
Oid *param_types; /* array of parameter type OIDs, or NULL */
|
Oid *param_types; /* array of parameter type OIDs, or NULL */
|
||||||
|
Loading…
Reference in New Issue
Block a user