Skip to content

Commit 1b67e0c

Browse files
tglsfdctglsfdc
committed
Fix core dump in duration logging for a V3-protocol Execute message
when what's being executed is a COMMIT or ROLLBACK. Per report from Sergey Koposov. Backpatch to 8.1; 8.0 and before don't have the bug due to lack of any logging at all here.
1 parent b475d25 commit 1b67e0c

File tree

1 file changed

+65
-47
lines changed

1 file changed

+65
-47
lines changed

‎src/backend/tcop/postgres.c

Lines changed: 65 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.497 2006/08/10 00:44:01 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/tcop/postgres.c,v 1.498 2006/08/13 22:18:08 tgl Exp $
1212
*
1313
* NOTES
1414
* this is the "main" module of the postgres backend and
@@ -1368,7 +1368,7 @@ exec_bind_message(StringInfo input_message)
13681368
PreparedStatement *pstmt;
13691369
Portal portal;
13701370
ParamListInfo params;
1371-
StringInfoData str;
1371+
StringInfoData bind_values_str;
13721372

13731373
pgstat_report_activity("<BIND>");
13741374

@@ -1385,7 +1385,7 @@ exec_bind_message(StringInfo input_message)
13851385
MemoryContextSwitchTo(MessageContext);
13861386

13871387
if (log_statement == LOGSTMT_ALL)
1388-
initStringInfo(&str);
1388+
initStringInfo(&bind_values_str);
13891389

13901390
/* Get the fixed part of the message */
13911391
portal_name = pq_getmsgstring(input_message);
@@ -1534,14 +1534,18 @@ exec_bind_message(StringInfo input_message)
15341534
else
15351535
pstring = pg_client_to_server(pbuf.data, plength);
15361536

1537-
params->params[paramno].value = OidInputFunctionCall(typinput,
1538-
pstring,
1539-
typioparam,
1540-
-1);
1537+
params->params[paramno].value =
1538+
OidInputFunctionCall(typinput,
1539+
pstring,
1540+
typioparam,
1541+
-1);
15411542

1543+
/* Log the parameter value if needed */
15421544
if (log_statement == LOGSTMT_ALL)
1543-
appendStringInfo(&str, "%s$%d = \"%s\"",
1544-
*str.data ? ", " : "", paramno + 1, pstring);
1545+
appendStringInfo(&bind_values_str, "%s$%d = \"%s\"",
1546+
bind_values_str.len ? ", " : "",
1547+
paramno + 1,
1548+
pstring);
15451549

15461550
/* Free result of encoding conversion, if any */
15471551
if (pstring && pstring != pbuf.data)
@@ -1574,6 +1578,8 @@ exec_bind_message(StringInfo input_message)
15741578
(errcode(ERRCODE_INVALID_BINARY_REPRESENTATION),
15751579
errmsg("incorrect binary data format in bind parameter %d",
15761580
paramno + 1)));
1581+
1582+
/* XXX TODO: convert value to text and log it */
15771583
}
15781584
else
15791585
{
@@ -1600,22 +1606,14 @@ exec_bind_message(StringInfo input_message)
16001606

16011607
if (log_statement == LOGSTMT_ALL)
16021608
{
1603-
if (*str.data)
1604-
ereport(LOG,
1605-
(errmsg("bind %s%s%s: %s",
1606-
*stmt_name ? stmt_name : "<unnamed>",
1607-
*portal->name ? "/" : "",
1608-
*portal->name ? portal->name : "",
1609-
pstmt->query_string ? pstmt->query_string : ""),
1610-
errdetail(str.data)));
1611-
else
1612-
ereport(LOG,
1613-
(errmsg("bind %s%s%s: %s",
1614-
*stmt_name ? stmt_name : "<unnamed>",
1615-
*portal->name ? "/" : "",
1616-
*portal->name ? portal->name : "",
1617-
pstmt->query_string ? pstmt->query_string : "")));
1618-
pfree(str.data);
1609+
ereport(LOG,
1610+
(errmsg("bind %s%s%s: %s",
1611+
*stmt_name ? stmt_name : "<unnamed>",
1612+
*portal->name ? "/" : "",
1613+
*portal->name ? portal->name : "",
1614+
pstmt->query_string ? pstmt->query_string : ""),
1615+
bind_values_str.len ? errdetail(bind_values_str.data) : 0));
1616+
pfree(bind_values_str.data);
16191617
}
16201618

16211619
/* Get the result format codes */
@@ -1685,8 +1683,11 @@ exec_execute_message(const char *portal_name, long max_rows)
16851683
Portal portal;
16861684
bool completed;
16871685
char completionTag[COMPLETION_TAG_BUFSIZE];
1686+
const char *sourceText = NULL;
1687+
const char *prepStmtName;
16881688
bool save_log_statement_stats = log_statement_stats;
1689-
bool execute_is_fetch = false;
1689+
bool is_xact_command;
1690+
bool execute_is_fetch;
16901691

16911692
/* Adjust destination to tell printtup.c what to do */
16921693
dest = whereToSendOutput;
@@ -1699,15 +1700,6 @@ exec_execute_message(const char *portal_name, long max_rows)
16991700
(errcode(ERRCODE_UNDEFINED_CURSOR),
17001701
errmsg("portal \"%s\" does not exist", portal_name)));
17011702

1702-
/*
1703-
* If we re-issue an Execute protocol request against an existing portal,
1704-
* then we are only fetching more rows rather than completely re-executing
1705-
* the query from the start. atStart is never reset for a v3 portal, so we
1706-
* are safe to use this check.
1707-
*/
1708-
if (!portal->atStart)
1709-
execute_is_fetch = true;
1710-
17111703
/*
17121704
* If the original query was a null string, just return
17131705
* EmptyQueryResponse.
@@ -1719,6 +1711,17 @@ exec_execute_message(const char *portal_name, long max_rows)
17191711
return;
17201712
}
17211713

1714+
/* Does the portal contain a transaction command? */
1715+
is_xact_command = IsTransactionStmtList(portal->parseTrees);
1716+
1717+
/*
1718+
* If we re-issue an Execute protocol request against an existing portal,
1719+
* then we are only fetching more rows rather than completely re-executing
1720+
* the query from the start. atStart is never reset for a v3 portal, so we
1721+
* are safe to use this check.
1722+
*/
1723+
execute_is_fetch = !portal->atStart;
1724+
17221725
/* Should we display the portal names here? */
17231726
if (execute_is_fetch)
17241727
{
@@ -1727,8 +1730,18 @@ exec_execute_message(const char *portal_name, long max_rows)
17271730
}
17281731
else if (portal->sourceText)
17291732
{
1730-
debug_query_string = portal->sourceText;
1731-
pgstat_report_activity(portal->sourceText);
1733+
/*
1734+
* We must copy the sourceText into MessageContext in case the
1735+
* portal is destroyed during finish_xact_command. Can avoid
1736+
* the copy if it's not an xact command, though.
1737+
*/
1738+
if (is_xact_command)
1739+
sourceText = pstrdup(portal->sourceText);
1740+
else
1741+
sourceText = portal->sourceText;
1742+
1743+
debug_query_string = sourceText;
1744+
pgstat_report_activity(sourceText);
17321745
}
17331746
else
17341747
{
@@ -1738,6 +1751,12 @@ exec_execute_message(const char *portal_name, long max_rows)
17381751

17391752
set_ps_display(portal->commandTag, false);
17401753

1754+
/* copy prepStmtName now too, in case portal is destroyed */
1755+
if (portal->prepStmtName)
1756+
prepStmtName = pstrdup(portal->prepStmtName);
1757+
else
1758+
prepStmtName = "<unnamed>";
1759+
17411760
/*
17421761
* We use save_log_statement_stats so ShowUsage doesn't report incorrect
17431762
* results because ResetUsage wasn't called.
@@ -1746,14 +1765,13 @@ exec_execute_message(const char *portal_name, long max_rows)
17461765
ResetUsage();
17471766

17481767
if (log_statement == LOGSTMT_ALL)
1749-
/* We have the portal, so output the source query. */
17501768
ereport(LOG,
17511769
(errmsg("execute %s%s%s%s: %s",
17521770
execute_is_fetch ? "fetch from " : "",
1753-
portal->prepStmtName ? portal->prepStmtName : "<unnamed>",
1754-
*portal->name ? "/" : "",
1755-
*portal->name ? portal->name : "",
1756-
portal->sourceText ? portal->sourceText : "")));
1771+
prepStmtName,
1772+
*portal_name ? "/" : "",
1773+
*portal_name ? portal_name : "",
1774+
sourceText ? sourceText : "")));
17571775

17581776
BeginCommand(portal->commandTag, dest);
17591777

@@ -1799,7 +1817,7 @@ exec_execute_message(const char *portal_name, long max_rows)
17991817

18001818
if (completed)
18011819
{
1802-
if (IsTransactionStmtList(portal->parseTrees))
1820+
if (is_xact_command)
18031821
{
18041822
/*
18051823
* If this was a transaction control statement, commit it. We
@@ -1861,10 +1879,10 @@ exec_execute_message(const char *portal_name, long max_rows)
18611879
(errmsg("duration: %ld.%03d ms execute %s%s%s%s: %s",
18621880
secs * 1000 + msecs, usecs % 1000,
18631881
execute_is_fetch ? "fetch from " : "",
1864-
portal->prepStmtName ? portal->prepStmtName : "<unnamed>",
1865-
*portal->name ? "/" : "",
1866-
*portal->name ? portal->name : "",
1867-
portal->sourceText ? portal->sourceText : "")));
1882+
prepStmtName,
1883+
*portal_name ? "/" : "",
1884+
*portal_name ? portal_name : "",
1885+
sourceText ? sourceText : "")));
18681886
}
18691887
}
18701888

0 commit comments

Comments
 (0)