Re: Cleaning up PREPARE query strings?

> This was already reported by Tom Lane on his first message, although his
> complaint was about execution time error reporting while this is during
> parse-analysis. However, I think that the exact same approach can be used to
> fixed both, either updating the position of every single element (which no one
> wants) or teaching the executor (and evidently the planstate) about a new
> "query offset" so that parser_errposition and executor_errposition report the
> correct location.

Maybe this is what you also mean, but I think it should be enough to
just tell Query
about the updated location = 0, and we don't need to touch anything else
( the raw parestree or the copy of it in plansource ). This will
ensure that the error
tracking is relying on the original raw parsetree, and consumers of the updated
statement query location and lengths are getting the updated values based on
the cleaned up query text that is stored in prepared statements.

I tested the below with the tests you have in v3 so far and it works.
I also tested
the error position and it looks correct.

Not sure if I am missing something.

```
--- a/src/backend/commands/prepare.c
+++ b/src/backend/commands/prepare.c
@@ -27,6 +27,7 @@
#include "commands/prepare.h"
#include "funcapi.h"
#include "nodes/nodeFuncs.h"
+#include "nodes/queryjumble.h"
#include "parser/parse_coerce.h"
#include "parser/parse_collate.h"
#include "parser/parse_expr.h"
@@ -64,6 +65,8 @@ PrepareQuery(ParseState *pstate, PrepareStmt *stmt,
Oid *argtypes = NULL;
int nargs;
List *query_list;
+ const char *new_query;
+ ListCell *lc;

/*
* Disallow empty-string statement name (conflicts with protocol-level
@@ -82,12 +85,31 @@ PrepareQuery(ParseState *pstate, PrepareStmt *stmt,
rawstmt->stmt = stmt->query;
rawstmt->stmt_location = stmt_location;
rawstmt->stmt_len = stmt_len;
+ new_query = pstate->p_sourcetext;
+
+ /*
+ * If we have a multi-statement string, confine the query to
the query we
+ * care about.
+ */
+ if (stmt_location >= 0)
+ {
+ const char *cleaned;
+ char *tmp;
+
+ cleaned = CleanQuerytext(pstate->p_sourcetext,
&rawstmt->stmt_location,
+
&rawstmt->stmt_len);
+
+ tmp = palloc(rawstmt->stmt_len + 1);
+ strlcpy(tmp, cleaned, rawstmt->stmt_len + 1);
+
+ new_query = tmp;
+ }

/*
* Create the CachedPlanSource before we do parse analysis,
since it needs
* to see the unmodified raw parse tree.
*/
- plansource = CreateCachedPlan(rawstmt, pstate->p_sourcetext,
+ plansource = CreateCachedPlan(rawstmt, new_query,

CreateCommandTag(stmt->query));

/* Transform list of TypeNames to array of type OIDs */
@@ -119,6 +141,17 @@ PrepareQuery(ParseState *pstate, PrepareStmt *stmt,
query_list = pg_analyze_and_rewrite_varparams(rawstmt,
pstate->p_sourcetext,

&argtypes, &nargs, NULL);

+ /*
+ * Set the stmt location to 0 and update stmt_len, since at
this point we
+ * only have the query we care about in case of a
multi-statement string.
+ */
+ foreach(lc, query_list)
+ {
+ Query *query = lfirst_node(Query, lc);
+
+ if (query->stmt_location > 0)
+ query->stmt_location = 0;
+ }

```

--
Sami Imseih
Amazon Web Services (AWS)