rdelaage
/
postgresql
mirror of https://git.postgresql.org/git/postgresql.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Adjust things so that the query_string of a cached plan and the sourceText of 

a portal are never NULL, but reliably provide the source text of the query.
It turns out that there was only one place that was really taking a short-cut,
which was the 'EXECUTE' utility statement.  That doesn't seem like a
sufficiently critical performance hotspot to justify not offering a guarantee
of validity of the portal source text.  Fix it to copy the source text over
from the cached plan.  Add Asserts in the places that set up cached plans and
portals to reject null source strings, and simplify a bunch of places that
formerly needed to guard against nulls.

There may be a few places that cons up statements for execution without
having any source text at all; I found one such in ConvertTriggerToFK().
It seems sufficient to inject a phony source string in such a case,
for instance
        ProcessUtility((Node *) atstmt,
                       "(generated ALTER TABLE ADD FOREIGN KEY command)",
                       NULL, false, None_Receiver, NULL);

We should take a second look at the usage of debug_query_string,
particularly the recently added current_query() SQL function.

ITAGAKI Takahiro and Tom Lane
This commit is contained in:
Tom Lane 2008-07-18 20:26:06 +00:00
parent 6cc88f0af5
commit a1c692358b
11 changed files with 66 additions and 65 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

7
src/backend/commands/portalcmds.c
View File

@ -14,7 +14,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.74 2008/05/12 20:01:59 alvherre Exp $
* $PostgreSQL: pgsql/src/backend/commands/portalcmds.c,v 1.75 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -68,7 +68,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
RequireTransactionChain(isTopLevel, "DECLARE CURSOR");
/*
* Create a portal and copy the plan into its memory context.
* Create a portal and copy the plan and queryString into its memory.
*/
portal = CreatePortal(cstmt->portalname, false, false);
@ -77,8 +77,7 @@ PerformCursorOpen(PlannedStmt *stmt, ParamListInfo params,
stmt = copyObject(stmt);
stmt->utilityStmt = NULL; /* make it look like plain SELECT */
if (queryString) /* copy the source text too for safety */
queryString = pstrdup(queryString);
queryString = pstrdup(queryString);
PortalDefineQuery(portal,
NULL,

20
src/backend/commands/prepare.c
View File

@ -10,7 +10,7 @@
* Copyright (c) 2002-2008, PostgreSQL Global Development Group
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.87 2008/05/12 20:01:59 alvherre Exp $
* $PostgreSQL: pgsql/src/backend/commands/prepare.c,v 1.88 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -174,6 +174,7 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString,
ParamListInfo paramLI = NULL;
EState *estate = NULL;
Portal portal;
char *query_string;
/* Look it up in the hash table */
entry = FetchPreparedStatement(stmt->name, true);
@ -203,6 +204,10 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString,
/* Don't display the portal in pg_cursors, it is for internal use only */
portal->visible = false;
/* Copy the plan's saved query string into the portal's memory */
query_string = MemoryContextStrdup(PortalGetHeapMemory(portal),
entry->plansource->query_string);
/*
* For CREATE TABLE / AS EXECUTE, we must make a copy of the stored query
* so that we can modify its destination (yech, but this has always been
@ -249,13 +254,9 @@ ExecuteQuery(ExecuteStmt *stmt, const char *queryString,
plan_list = cplan->stmt_list;
}
/*
* Note: we don't bother to copy the source query string into the portal.
* Any errors it might be useful for will already have been reported.
*/
PortalDefineQuery(portal,
NULL,
NULL,
query_string,
entry->plansource->commandTag,
plan_list,
cplan);
@ -777,12 +778,7 @@ pg_prepared_statement(PG_FUNCTION_ARGS)
MemSet(nulls, 0, sizeof(nulls));
values[0] = CStringGetTextDatum(prep_stmt->stmt_name);
if (prep_stmt->plansource->query_string == NULL)
nulls[1] = true;
else
values[1] = CStringGetTextDatum(prep_stmt->plansource->query_string);
values[1] = CStringGetTextDatum(prep_stmt->plansource->query_string);
values[2] = TimestampTzGetDatum(prep_stmt->prepare_time);
values[3] = build_regtype_array(prep_stmt->plansource->param_types,
prep_stmt->plansource->num_params);

5
src/backend/commands/trigger.c
View File

@ -7,7 +7,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.235 2008/07/13 20:45:47 tgl Exp $
* $PostgreSQL: pgsql/src/backend/commands/trigger.c,v 1.236 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -728,7 +728,8 @@ ConvertTriggerToFK(CreateTrigStmt *stmt, Oid funcoid)
/* ... and execute it */
ProcessUtility((Node *) atstmt,
NULL, NULL, false, None_Receiver, NULL);
"(generated ALTER TABLE ADD FOREIGN KEY command)",
NULL, false, None_Receiver, NULL);
/* Remove the matched item from the list */
info_list = list_delete_ptr(info_list, info);

8
src/backend/executor/spi.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.196 2008/06/01 17:32:48 tgl Exp $
* $PostgreSQL: pgsql/src/backend/executor/spi.c,v 1.197 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -1057,10 +1057,8 @@ SPI_cursor_open_internal(const char *name, SPIPlanPtr plan,
*/
oldcontext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
/* Copy the plan's query string, if available, into the portal */
query_string = plansource->query_string;
if (query_string)
query_string = pstrdup(query_string);
/* Copy the plan's query string into the portal */
query_string = pstrdup(plansource->query_string);
/* If the plan has parameters, copy them into the portal */
if (plan->nargs > 0)

9
src/backend/parser/analyze.c
View File

@ -17,7 +17,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.372 2008/06/19 00:46:04 alvherre Exp $
* $PostgreSQL: pgsql/src/backend/parser/analyze.c,v 1.373 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -72,9 +72,6 @@ static bool check_parameter_resolution_walker(Node *node,
* parse_analyze
* Analyze a raw parse tree and transform it to Query form.
*
* If available, pass the source text from which the raw parse tree was
* generated; it's OK to pass NULL if this is not available.
*
* Optionally, information about $n parameter types can be supplied.
* References to $n indexes not defined by paramTypes[] are disallowed.
*
@ -89,6 +86,8 @@ parse_analyze(Node *parseTree, const char *sourceText,
ParseState *pstate = make_parsestate(NULL);
Query *query;
Assert(sourceText != NULL); /* required as of 8.4 */
pstate->p_sourcetext = sourceText;
pstate->p_paramtypes = paramTypes;
pstate->p_numparams = numParams;
@ -115,6 +114,8 @@ parse_analyze_varparams(Node *parseTree, const char *sourceText,
ParseState *pstate = make_parsestate(NULL);
Query *query;
Assert(sourceText != NULL); /* required as of 8.4 */
pstate->p_sourcetext = sourceText;
pstate->p_paramtypes = *paramTypes;
pstate->p_numparams = *numParams;

32
src/backend/tcop/postgres.c
View File

@ -8,7 +8,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.553 2008/05/15 00:17:40 tgl Exp $
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.554 2008/07/18 20:26:06 tgl Exp $
*
* NOTES
* this is the "main" module of the postgres backend and
@ -1388,9 +1388,9 @@ exec_bind_message(StringInfo input_message)
/*
* Report query to various monitoring facilities.
*/
debug_query_string = psrc->query_string ? psrc->query_string : "<BIND>";
debug_query_string = psrc->query_string;
pgstat_report_activity(debug_query_string);
pgstat_report_activity(psrc->query_string);
set_ps_display("BIND", false);
@ -1466,10 +1466,8 @@ exec_bind_message(StringInfo input_message)
*/
oldContext = MemoryContextSwitchTo(PortalGetHeapMemory(portal));
/* Copy the plan's query string, if available, into the portal */
query_string = psrc->query_string;
if (query_string)
query_string = pstrdup(query_string);
/* Copy the plan's query string into the portal */
query_string = pstrdup(psrc->query_string);
/* Likewise make a copy of the statement name, unless it's unnamed */
if (stmt_name[0])
@ -1714,7 +1712,7 @@ exec_bind_message(StringInfo input_message)
*stmt_name ? stmt_name : "<unnamed>",
*portal_name ? "/" : "",
*portal_name ? portal_name : "",
psrc->query_string ? psrc->query_string : "<source not stored>"),
psrc->query_string),
errhidestmt(true),
errdetail_params(params)));
break;
@ -1780,7 +1778,7 @@ exec_execute_message(const char *portal_name, long max_rows)
*/
if (is_xact_command)
{
sourceText = portal->sourceText ? pstrdup(portal->sourceText) : NULL;
sourceText = pstrdup(portal->sourceText);
if (portal->prepStmtName)
prepStmtName = pstrdup(portal->prepStmtName);
else
@ -1805,9 +1803,9 @@ exec_execute_message(const char *portal_name, long max_rows)
/*
* Report query to various monitoring facilities.
*/
debug_query_string = sourceText ? sourceText : "<EXECUTE>";
debug_query_string = sourceText;
pgstat_report_activity(debug_query_string);
pgstat_report_activity(sourceText);
set_ps_display(portal->commandTag, false);
@ -1840,15 +1838,14 @@ exec_execute_message(const char *portal_name, long max_rows)
if (check_log_statement(portal->stmts))
{
ereport(LOG,
(errmsg("%s %s%s%s%s%s",
(errmsg("%s %s%s%s: %s",
execute_is_fetch ?
_("execute fetch from") :
_("execute"),
prepStmtName,
*portal_name ? "/" : "",
*portal_name ? portal_name : "",
sourceText ? ": " : "",
sourceText ? sourceText : ""),
sourceText),
errhidestmt(true),
errdetail_params(portalParams)));
was_logged = true;
@ -1924,7 +1921,7 @@ exec_execute_message(const char *portal_name, long max_rows)
break;
case 2:
ereport(LOG,
(errmsg("duration: %s ms %s %s%s%s%s%s",
(errmsg("duration: %s ms %s %s%s%s: %s",
msec_str,
execute_is_fetch ?
_("execute fetch from") :
@ -1932,8 +1929,7 @@ exec_execute_message(const char *portal_name, long max_rows)
prepStmtName,
*portal_name ? "/" : "",
*portal_name ? portal_name : "",
sourceText ? ": " : "",
sourceText ? sourceText : ""),
sourceText),
errhidestmt(true),
errdetail_params(portalParams)));
break;
@ -2049,7 +2045,7 @@ errdetail_execute(List *raw_parsetree_list)
PreparedStatement *pstmt;
pstmt = FetchPreparedStatement(stmt->name, false);
if (pstmt && pstmt->plansource->query_string)
if (pstmt)
{
errdetail("prepare: %s", pstmt->plansource->query_string);
return 0;

10
src/backend/tcop/utility.c
View File

@ -10,7 +10,7 @@
*
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.294 2008/06/15 01:25:54 tgl Exp $
* $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.295 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -218,13 +218,17 @@ check_xact_readonly(Node *parsetree)
* general utility function invoker
*
* parsetree: the parse tree for the utility statement
* queryString: original source text of command (NULL if not available)
* queryString: original source text of command
* params: parameters to use during execution
* isTopLevel: true if executing a "top level" (interactively issued) command
* dest: where to send results
* completionTag: points to a buffer of size COMPLETION_TAG_BUFSIZE
* in which to store a command completion status string.
*
* Notes: as of PG 8.4, caller MUST supply a queryString; it is not
* allowed anymore to pass NULL. (If you really don't have source text,
* you can pass a constant string, perhaps "(query not available)".)
*
* completionTag is only set nonempty if we want to return a nondefault status.
*
* completionTag may be NULL if caller doesn't want a status string.
@ -237,6 +241,8 @@ ProcessUtility(Node *parsetree,
DestReceiver *dest,
char *completionTag)
{
Assert(queryString != NULL); /* required as of 8.4 */
check_xact_readonly(parsetree);
if (completionTag)

11
src/backend/utils/cache/plancache.c vendored
View File

@ -33,7 +33,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.18 2008/05/12 20:02:02 alvherre Exp $
* $PostgreSQL: pgsql/src/backend/utils/cache/plancache.c,v 1.19 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -104,8 +104,7 @@ InitPlanCache(void)
* about all that we do here is copy it into permanent storage.
*
* raw_parse_tree: output of raw_parser()
* query_string: original query text (can be NULL if not available, but
* that is discouraged because it degrades error message quality)
* query_string: original query text (as of PG 8.4, must not be NULL)
* commandTag: compile-time-constant tag for query, or NULL if empty query
* param_types: array of parameter type OIDs, or NULL if none
* num_params: number of parameters
@ -130,6 +129,8 @@ CreateCachedPlan(Node *raw_parse_tree,
MemoryContext source_context;
MemoryContext oldcxt;
Assert(query_string != NULL); /* required as of 8.4 */
/*
* Make a dedicated memory context for the CachedPlanSource and its
* subsidiary data. We expect it can be pretty small.
@ -152,7 +153,7 @@ CreateCachedPlan(Node *raw_parse_tree,
oldcxt = MemoryContextSwitchTo(source_context);
plansource = (CachedPlanSource *) palloc(sizeof(CachedPlanSource));
plansource->raw_parse_tree = copyObject(raw_parse_tree);
plansource->query_string = query_string ? pstrdup(query_string) : NULL;
plansource->query_string = pstrdup(query_string);
plansource->commandTag = commandTag; /* no copying needed */
if (num_params > 0)
{
@ -228,6 +229,8 @@ FastCreateCachedPlan(Node *raw_parse_tree,
OverrideSearchPath *search_path;
MemoryContext oldcxt;
Assert(query_string != NULL); /* required as of 8.4 */
/*
* Fetch current search_path into given context, but do any recalculation
* work required in caller's context.

18
src/backend/utils/mmgr/portalmem.c
View File

@ -12,7 +12,7 @@
* Portions Copyright (c) 1994, Regents of the University of California
*
* IDENTIFICATION
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.110 2008/05/12 00:00:52 alvherre Exp $
* $PostgreSQL: pgsql/src/backend/utils/mmgr/portalmem.c,v 1.111 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -271,7 +271,11 @@ CreateNewPortal(void)
* PortalDefineQuery
* A simple subroutine to establish a portal's query.
*
* Notes: commandTag shall be NULL if and only if the original query string
* Notes: as of PG 8.4, caller MUST supply a sourceText string; it is not
* allowed anymore to pass NULL. (If you really don't have source text,
* you can pass a constant string, perhaps "(query not available)".)
*
* commandTag shall be NULL if and only if the original query string
* (before rewriting) was an empty string. Also, the passed commandTag must
* be a pointer to a constant string, since it is not copied.
*
@ -284,7 +288,7 @@ CreateNewPortal(void)
* copying them into the portal's heap context.
*
* The caller is also responsible for ensuring that the passed prepStmtName
* and sourceText (if not NULL) have adequate lifetime.
* (if not NULL) and sourceText have adequate lifetime.
*
* NB: this function mustn't do much beyond storing the passed values; in
* particular don't do anything that risks elog(ERROR). If that were to
@ -302,7 +306,8 @@ PortalDefineQuery(Portal portal,
AssertArg(PortalIsValid(portal));
AssertState(portal->status == PORTAL_NEW);
Assert(commandTag != NULL || stmts == NIL);
AssertArg(sourceText != NULL);
AssertArg(commandTag != NULL || stmts == NIL);
portal->prepStmtName = prepStmtName;
portal->sourceText = sourceText;
@ -927,10 +932,7 @@ pg_cursor(PG_FUNCTION_ARGS)
MemSet(nulls, 0, sizeof(nulls));
values[0] = CStringGetTextDatum(portal->name);
if (!portal->sourceText)
nulls[1] = true;
else
values[1] = CStringGetTextDatum(portal->sourceText);
values[1] = CStringGetTextDatum(portal->sourceText);
values[2] = BoolGetDatum(portal->cursorOptions & CURSOR_OPT_HOLD);
values[3] = BoolGetDatum(portal->cursorOptions & CURSOR_OPT_BINARY);
values[4] = BoolGetDatum(portal->cursorOptions & CURSOR_OPT_SCROLL);

7
src/include/utils/plancache.h
View File

@ -8,7 +8,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $PostgreSQL: pgsql/src/include/utils/plancache.h,v 1.11 2008/01/01 19:45:59 momjian Exp $
* $PostgreSQL: pgsql/src/include/utils/plancache.h,v 1.12 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -20,8 +20,7 @@
/*
* CachedPlanSource represents the portion of a cached plan that persists
* across invalidation/replan cycles. It stores a raw parse tree (required),
* the original source text (optional, but highly recommended to improve
* error reports), and adjunct data.
* the original source text (also required, as of 8.4), and adjunct data.
*
* Normally, both the struct itself and the subsidiary data live in the
* context denoted by the context field, while the linked-to CachedPlan, if
@ -47,7 +46,7 @@
typedef struct CachedPlanSource
{
Node *raw_parse_tree; /* output of raw_parser() */
char *query_string; /* text of query, or NULL */
char *query_string; /* text of query (as of 8.4, never NULL) */
const char *commandTag; /* command tag (a constant!), or NULL */
Oid *param_types; /* array of parameter type OIDs, or NULL */
int num_params; /* length of param_types array */

4
src/include/utils/portal.h
View File

@ -39,7 +39,7 @@
* Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.78 2008/01/01 19:45:59 momjian Exp $
* $PostgreSQL: pgsql/src/include/utils/portal.h,v 1.79 2008/07/18 20:26:06 tgl Exp $
*
*-------------------------------------------------------------------------
*/
@ -123,7 +123,7 @@ typedef struct PortalData
*/
/* The query or queries the portal will execute */
const char *sourceText; /* text of query, if known (may be NULL) */
const char *sourceText; /* text of query (as of 8.4, never NULL) */
const char *commandTag; /* command tag for original query */
List *stmts; /* PlannedStmts and/or utility statements */
CachedPlan *cplan; /* CachedPlan, if stmts are from one */