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 commits a73c9dbab0165b3395dfe8a44a7dfd16166963c4 and
2e9650cbcff8c8fb0d9ef807c73a44f241822eee, 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.

diff --git a/src/backend/executor/spi.c b/src/backend/executor/spi.c
index f57299bcc7573e04a738013c34c11cc55c2a8c35..98cf744f89cd94edfb64f40670e4d26a74ad7db8 100644 (file)
--- a/src/backend/executor/spi.c
+++ b/src/backend/executor/spi.c
@@ -1942,7 +1942,9 @@ _SPI_execute_plan(SPIPlanPtr plan, ParamListInfo paramLI,
                         * Parameter datatypes are driven by parserSetup hook if provided,
                         * 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);
                                stmt_list = pg_analyze_and_rewrite_params(parsetree,

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index f7f1081258afeb63f61d4b7c804b7971039229dc..5bbd3134830e8bba0a78ebb2e22cc21b7578a62d 100644 (file)
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -287,17 +287,13 @@ transformStmt(ParseState *pstate, Node *parseTree)
  *             Returns true if a snapshot must be set before doing parse analysis
  *             on the given raw parse tree.
  *
- * Classification here should match transformStmt(); but we also have to
- * allow a NULL input (for Parse/Bind of an empty query string).
+ * Classification here should match transformStmt().
  */
 bool
 analyze_requires_snapshot(Node *parseTree)
 {
        bool            result;
 
-       if (parseTree == NULL)
-               return false;
-
        switch (nodeTag(parseTree))
        {
                        /*

diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 906e7ef1d7ee0b1929b5b0423c8c4da9b253182b..6a0d7996e903003efa99d9a82d249684be9ea20a 100644 (file)
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -1547,7 +1547,9 @@ exec_bind_message(StringInfo input_message)
         * snapshot active till we're done, so that plancache.c doesn't have to
         * 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());
                snapshot_set = true;

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 81e832506afd8b87b7f613343c9c0b5801cc3ed7..d41c692bcbe982e9bdcda391598b108ec1f23e64 100644 (file)
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -2184,9 +2184,6 @@ GetCommandLogLevel(Node *parsetree)
 {
        LogStmtLevel lev;
 
-       if (parsetree == NULL)
-               return LOGSTMT_ALL;
-
        switch (nodeTag(parsetree))
        {
                        /* raw plannable queries */
@@ -2290,7 +2287,7 @@ GetCommandLogLevel(Node *parsetree)
 
                                /* Look through an EXECUTE to the referenced stmt */
                                ps = FetchPreparedStatement(stmt->name, false);
-                               if (ps)
+                               if (ps && ps->plansource->raw_parse_tree)
                                        lev = GetCommandLogLevel(ps->plansource->raw_parse_tree);
                                else
                                        lev = LOGSTMT_ALL;

diff --git a/src/backend/utils/cache/plancache.c b/src/backend/utils/cache/plancache.c
index 4af2138ce1f388ac6c22a5c1eeae7316fe05bb13..09079e22f274e1eebbcd5f10950719f8247727e7 100644 (file)
--- a/src/backend/utils/cache/plancache.c
+++ b/src/backend/utils/cache/plancache.c
@@ -136,7 +136,7 @@ InitPlanCache(void)
  * Once constructed, the cached plan can be made longer-lived, if needed,
  * 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
  * commandTag: compile-time-constant tag for query, or NULL if empty query
  */
@@ -218,7 +218,7 @@ CreateCachedPlan(Node *raw_parse_tree,
  * invalidation, so plan use must be completed in the current transaction,
  * 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
  * commandTag: compile-time-constant tag for query, or NULL if empty query
  */
@@ -644,7 +644,9 @@ RevalidateCachedQuery(CachedPlanSource *plansource)
         * the cache.
         */
        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,
                                                                                          plansource->query_string,
                                                                                          plansource->parserSetup,
@@ -883,6 +885,7 @@ BuildCachedPlan(CachedPlanSource *plansource, List *qlist,
         */
        snapshot_set = false;
        if (!ActiveSnapshotSet() &&
+               plansource->raw_parse_tree &&
                analyze_requires_snapshot(plansource->raw_parse_tree))
        {
                PushActiveSnapshot(GetTransactionSnapshot());

diff --git a/src/include/utils/plancache.h b/src/include/utils/plancache.h
index 382870c81859437ea562eaab85188045036afbb7..f44dd49534bf9e1b46c4e03bb9085703495bcf20 100644 (file)
--- a/src/include/utils/plancache.h
+++ b/src/include/utils/plancache.h
@@ -76,7 +76,7 @@
 typedef struct CachedPlanSource
 {
        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 *commandTag;         /* command tag (a constant!), or NULL */
        Oid                *param_types;        /* array of parameter type OIDs, or NULL */