Re: BUG #17053: Memory corruption in parser on prepared query reuse

PG Bug reporting form <noreply@postgresql.org> writes:
> The code is fairly short, the core of the issue is that the prepared query
> `q1` is executed twice and it somehow messes up with the parser because of
> the `CHECK` clause.

Thanks for the report!  The core of the problem is that where
transformExpr does this:

        case T_NullTest:
            {
                NullTest   *n = (NullTest *) expr;

                n->arg = (Expr *) transformExprRecurse(pstate, (Node *) n->arg);

in the case of a prepared statement, it's scribbling on the same NullTest
node that's in the plan cache.  So the next time we try to execute that
cache entry, the arg pointer is now pointing at garbage, or at least not
at the untransformed argument it should be pointing at.

Ideally, maybe, the parser wouldn't ever modify its input.  However,
this code fragment has got LOTS of company, so making that happen has
never seemed too practical.  So what we generally do, before invoking
the parser (or planner), is a quick copyObject() on any node tree that
might be coming from cache.

The proximate cause of this problem is that CREATE/ALTER DOMAIN did
not get that memo.  Looking around in typecmds.c, I noted that
CREATE DOMAIN ... DEFAULT ... and ALTER DOMAIN SET DEFAULT have the
same kind of bug.

The attached patch seems to fix it.  I wonder though if we ought to
move the copying to someplace more centralized.  I cannot escape the
suspicion that there are more of these creepy-crawlies in other
poorly-tested utility statements.

BTW, a note for future testing: you can set up this failure using
pgbench, no custom client code required.  I tested stuff like this:

$ cat bug17053b.sql
DROP DOMAIN IF EXISTS schema_meta;
CREATE DOMAIN schema_meta AS bool DEFAULT (42 IS NOT NULL);
$ pgbench -n -M prepared -f bug17053b.sql regression
NOTICE:  type "schema_meta" does not exist, skipping
pgbench: error: client 0 script 0 aborted in command 1 query 0: ERROR:  unrecognized node type: 2139062143

            regards, tom lane

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 58ec65c6af..9a1b261d85 100644
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -891,10 +891,12 @@ DefineDomain(CreateDomainStmt *stmt)
                     pstate = make_parsestate(NULL);

                     /*
-                     * Cook the constr->raw_expr into an expression. Note:
-                     * name is strictly for error message
+                     * Cook the constr->raw_expr into an expression; copy it
+                     * in case the input is in plan cache.  Note: name is used
+                     * only for error messages.
                      */
-                    defaultExpr = cookDefault(pstate, constr->raw_expr,
+                    defaultExpr = cookDefault(pstate,
+                                              copyObject(constr->raw_expr),
                                               basetypeoid,
                                               basetypeMod,
                                               domainName,
@@ -2623,10 +2625,10 @@ AlterDomainDefault(List *names, Node *defaultRaw)
         pstate = make_parsestate(NULL);

         /*
-         * Cook the colDef->raw_expr into an expression. Note: Name is
-         * strictly for error message
+         * Cook the raw default into an expression; copy it in case the input
+         * is in plan cache.  Note: name is used only for error messages.
          */
-        defaultExpr = cookDefault(pstate, defaultRaw,
+        defaultExpr = cookDefault(pstate, copyObject(defaultRaw),
                                   typTup->typbasetype,
                                   typTup->typtypmod,
                                   NameStr(typTup->typname),
@@ -3508,7 +3510,12 @@ domainAddConstraint(Oid domainOid, Oid domainNamespace, Oid baseTypeOid,
     pstate->p_pre_columnref_hook = replace_domain_constraint_value;
     pstate->p_ref_hook_state = (void *) domVal;

-    expr = transformExpr(pstate, constr->raw_expr, EXPR_KIND_DOMAIN_CHECK);
+    /*
+     * Transform the expression; first we must copy the input, in case it's in
+     * plan cache.
+     */
+    expr = transformExpr(pstate, copyObject(constr->raw_expr),
+                         EXPR_KIND_DOMAIN_CHECK);

     /*
      * Make sure it yields a boolean result.