Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit00d4f2a

Browse files
committed
Improve connection-failure error handling in contrib/postgres_fdw.
postgres_fdw tended to say "unknown error" if it tried to execute a commandon an already-dead connection, because some paths in libpq just return anull PGresult for such cases. Out-of-memory might result in that, too.To fix, pass the PGconn to pgfdw_report_error, and look at itsPQerrorMessage() string if we can't get anything out of the PGresult.Also, fix the transaction-exit logic to reliably drop a dead connection.It was attempting to do that already, but it assumed that only connectioncache entries with xact_depth > 0 needed to be examined. The folly in thatis that if we fail while issuing START TRANSACTION, we'll not have bumpedxact_depth. (At least for the case I was testing, this fix masks theother problem; but it still seems like a good idea to have the PGconnfallback logic.)Per investigation of bug #9087 from Craig Lucas. Backpatch to 9.3 wherethis code was introduced.
1 parent489e6ac commit00d4f2a

File tree

3 files changed

+100
-85
lines changed

3 files changed

+100
-85
lines changed

‎contrib/postgres_fdw/connection.c

Lines changed: 85 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -355,7 +355,7 @@ do_sql_command(PGconn *conn, const char *sql)
355355

356356
res=PQexec(conn,sql);
357357
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
358-
pgfdw_report_error(ERROR,res, true,sql);
358+
pgfdw_report_error(ERROR,res,conn,true,sql);
359359
PQclear(res);
360360
}
361361

@@ -454,6 +454,7 @@ GetPrepStmtNumber(PGconn *conn)
454454
*
455455
* elevel: error level to use (typically ERROR, but might be less)
456456
* res: PGresult containing the error
457+
* conn: connection we did the query on
457458
* clear: if true, PQclear the result (otherwise caller will handle it)
458459
* sql: NULL, or text of remote command we tried to execute
459460
*
@@ -462,7 +463,8 @@ GetPrepStmtNumber(PGconn *conn)
462463
* marked with have_error = true.
463464
*/
464465
void
465-
pgfdw_report_error(intelevel,PGresult*res,boolclear,constchar*sql)
466+
pgfdw_report_error(intelevel,PGresult*res,PGconn*conn,
467+
boolclear,constchar*sql)
466468
{
467469
/* If requested, PGresult must be released before leaving this function. */
468470
PG_TRY();
@@ -483,6 +485,14 @@ pgfdw_report_error(int elevel, PGresult *res, bool clear, const char *sql)
483485
else
484486
sqlstate=ERRCODE_CONNECTION_FAILURE;
485487

488+
/*
489+
* If we don't get a message from the PGresult, try the PGconn. This
490+
* is needed because for connection-level failures, PQexec may just
491+
* return NULL, not a PGresult at all.
492+
*/
493+
if (message_primary==NULL)
494+
message_primary=PQerrorMessage(conn);
495+
486496
ereport(elevel,
487497
(errcode(sqlstate),
488498
message_primary ?errmsg_internal("%s",message_primary) :
@@ -525,83 +535,88 @@ pgfdw_xact_callback(XactEvent event, void *arg)
525535
{
526536
PGresult*res;
527537

528-
/*We only care about connections withopenremote transactions */
529-
if (entry->conn==NULL||entry->xact_depth==0)
538+
/*Ignore cache entry if noopenconnection right now */
539+
if (entry->conn==NULL)
530540
continue;
531541

532-
elog(DEBUG3,"closing remote transaction on connection %p",
533-
entry->conn);
534-
535-
switch (event)
542+
/* If it has an open remote transaction, try to close it */
543+
if (entry->xact_depth>0)
536544
{
537-
caseXACT_EVENT_PRE_COMMIT:
538-
/* Commit all remote transactions during pre-commit */
539-
do_sql_command(entry->conn,"COMMIT TRANSACTION");
540-
541-
/*
542-
* If there were any errors in subtransactions, and we made
543-
* prepared statements, do a DEALLOCATE ALL to make sure we
544-
* get rid of all prepared statements.This is annoying and
545-
* not terribly bulletproof, but it's probably not worth
546-
* trying harder.
547-
*
548-
* DEALLOCATE ALL only exists in 8.3 and later, so this
549-
* constrains how old a server postgres_fdw can communicate
550-
* with. We intentionally ignore errors in the DEALLOCATE, so
551-
* that we can hobble along to some extent with older servers
552-
* (leaking prepared statements as we go; but we don't really
553-
* support update operations pre-8.3 anyway).
554-
*/
555-
if (entry->have_prep_stmt&&entry->have_error)
556-
{
557-
res=PQexec(entry->conn,"DEALLOCATE ALL");
558-
PQclear(res);
559-
}
560-
entry->have_prep_stmt= false;
561-
entry->have_error= false;
562-
break;
563-
caseXACT_EVENT_PRE_PREPARE:
564-
565-
/*
566-
* We disallow remote transactions that modified anything,
567-
* since it's not really reasonable to hold them open until
568-
* the prepared transaction is committed. For the moment,
569-
* throw error unconditionally; later we might allow read-only
570-
* cases. Note that the error will cause us to come right
571-
* back here with event == XACT_EVENT_ABORT, so we'll clean up
572-
* the connection state at that point.
573-
*/
574-
ereport(ERROR,
575-
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
576-
errmsg("cannot prepare a transaction that modified remote tables")));
577-
break;
578-
caseXACT_EVENT_COMMIT:
579-
caseXACT_EVENT_PREPARE:
580-
/* Should not get here -- pre-commit should have handled it */
581-
elog(ERROR,"missed cleaning up connection during pre-commit");
582-
break;
583-
caseXACT_EVENT_ABORT:
584-
/* Assume we might have lost track of prepared statements */
585-
entry->have_error= true;
586-
/* If we're aborting, abort all remote transactions too */
587-
res=PQexec(entry->conn,"ABORT TRANSACTION");
588-
/* Note: can't throw ERROR, it would be infinite loop */
589-
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
590-
pgfdw_report_error(WARNING,res, true,
591-
"ABORT TRANSACTION");
592-
else
593-
{
594-
PQclear(res);
595-
/* As above, make sure we've cleared any prepared stmts */
545+
elog(DEBUG3,"closing remote transaction on connection %p",
546+
entry->conn);
547+
548+
switch (event)
549+
{
550+
caseXACT_EVENT_PRE_COMMIT:
551+
/* Commit all remote transactions during pre-commit */
552+
do_sql_command(entry->conn,"COMMIT TRANSACTION");
553+
554+
/*
555+
* If there were any errors in subtransactions, and we
556+
* made prepared statements, do a DEALLOCATE ALL to make
557+
* sure we get rid of all prepared statements. This is
558+
* annoying and not terribly bulletproof, but it's
559+
* probably not worth trying harder.
560+
*
561+
* DEALLOCATE ALL only exists in 8.3 and later, so this
562+
* constrains how old a server postgres_fdw can
563+
* communicate with. We intentionally ignore errors in
564+
* the DEALLOCATE, so that we can hobble along to some
565+
* extent with older servers (leaking prepared statements
566+
* as we go; but we don't really support update operations
567+
* pre-8.3 anyway).
568+
*/
596569
if (entry->have_prep_stmt&&entry->have_error)
597570
{
598571
res=PQexec(entry->conn,"DEALLOCATE ALL");
599572
PQclear(res);
600573
}
601574
entry->have_prep_stmt= false;
602575
entry->have_error= false;
603-
}
604-
break;
576+
break;
577+
caseXACT_EVENT_PRE_PREPARE:
578+
579+
/*
580+
* We disallow remote transactions that modified anything,
581+
* since it's not very reasonable to hold them open until
582+
* the prepared transaction is committed. For the moment,
583+
* throw error unconditionally; later we might allow
584+
* read-only cases. Note that the error will cause us to
585+
* come right back here with event == XACT_EVENT_ABORT, so
586+
* we'll clean up the connection state at that point.
587+
*/
588+
ereport(ERROR,
589+
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
590+
errmsg("cannot prepare a transaction that modified remote tables")));
591+
break;
592+
caseXACT_EVENT_COMMIT:
593+
caseXACT_EVENT_PREPARE:
594+
/* Pre-commit should have closed the open transaction */
595+
elog(ERROR,"missed cleaning up connection during pre-commit");
596+
break;
597+
caseXACT_EVENT_ABORT:
598+
/* Assume we might have lost track of prepared statements */
599+
entry->have_error= true;
600+
/* If we're aborting, abort all remote transactions too */
601+
res=PQexec(entry->conn,"ABORT TRANSACTION");
602+
/* Note: can't throw ERROR, it would be infinite loop */
603+
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
604+
pgfdw_report_error(WARNING,res,entry->conn, true,
605+
"ABORT TRANSACTION");
606+
else
607+
{
608+
PQclear(res);
609+
/* As above, make sure to clear any prepared stmts */
610+
if (entry->have_prep_stmt&&entry->have_error)
611+
{
612+
res=PQexec(entry->conn,"DEALLOCATE ALL");
613+
PQclear(res);
614+
}
615+
entry->have_prep_stmt= false;
616+
entry->have_error= false;
617+
}
618+
break;
619+
}
605620
}
606621

607622
/* Reset state to show we're out of a transaction */
@@ -689,7 +704,7 @@ pgfdw_subxact_callback(SubXactEvent event, SubTransactionId mySubid,
689704
curlevel,curlevel);
690705
res=PQexec(entry->conn,sql);
691706
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
692-
pgfdw_report_error(WARNING,res, true,sql);
707+
pgfdw_report_error(WARNING,res,entry->conn,true,sql);
693708
else
694709
PQclear(res);
695710
}

‎contrib/postgres_fdw/postgres_fdw.c

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1040,7 +1040,7 @@ postgresReScanForeignScan(ForeignScanState *node)
10401040
*/
10411041
res=PQexec(fsstate->conn,sql);
10421042
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
1043-
pgfdw_report_error(ERROR,res, true,sql);
1043+
pgfdw_report_error(ERROR,res,fsstate->conn,true,sql);
10441044
PQclear(res);
10451045

10461046
/* Now force a fresh FETCH. */
@@ -1374,7 +1374,7 @@ postgresExecForeignInsert(EState *estate,
13741374
0);
13751375
if (PQresultStatus(res)!=
13761376
(fmstate->has_returning ?PGRES_TUPLES_OK :PGRES_COMMAND_OK))
1377-
pgfdw_report_error(ERROR,res, true,fmstate->query);
1377+
pgfdw_report_error(ERROR,res,fmstate->conn,true,fmstate->query);
13781378

13791379
/* Check number of rows affected, and fetch RETURNING tuple if any */
13801380
if (fmstate->has_returning)
@@ -1444,7 +1444,7 @@ postgresExecForeignUpdate(EState *estate,
14441444
0);
14451445
if (PQresultStatus(res)!=
14461446
(fmstate->has_returning ?PGRES_TUPLES_OK :PGRES_COMMAND_OK))
1447-
pgfdw_report_error(ERROR,res, true,fmstate->query);
1447+
pgfdw_report_error(ERROR,res,fmstate->conn,true,fmstate->query);
14481448

14491449
/* Check number of rows affected, and fetch RETURNING tuple if any */
14501450
if (fmstate->has_returning)
@@ -1514,7 +1514,7 @@ postgresExecForeignDelete(EState *estate,
15141514
0);
15151515
if (PQresultStatus(res)!=
15161516
(fmstate->has_returning ?PGRES_TUPLES_OK :PGRES_COMMAND_OK))
1517-
pgfdw_report_error(ERROR,res, true,fmstate->query);
1517+
pgfdw_report_error(ERROR,res,fmstate->conn,true,fmstate->query);
15181518

15191519
/* Check number of rows affected, and fetch RETURNING tuple if any */
15201520
if (fmstate->has_returning)
@@ -1563,7 +1563,7 @@ postgresEndForeignModify(EState *estate,
15631563
*/
15641564
res=PQexec(fmstate->conn,sql);
15651565
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
1566-
pgfdw_report_error(ERROR,res, true,sql);
1566+
pgfdw_report_error(ERROR,res,fmstate->conn,true,sql);
15671567
PQclear(res);
15681568
fmstate->p_name=NULL;
15691569
}
@@ -1800,7 +1800,7 @@ get_remote_estimate(const char *sql, PGconn *conn,
18001800
*/
18011801
res=PQexec(conn,sql);
18021802
if (PQresultStatus(res)!=PGRES_TUPLES_OK)
1803-
pgfdw_report_error(ERROR,res, false,sql);
1803+
pgfdw_report_error(ERROR,res,conn,false,sql);
18041804

18051805
/*
18061806
* Extract cost numbers for topmost plan node.Note we search for a
@@ -1934,7 +1934,7 @@ create_cursor(ForeignScanState *node)
19341934
res=PQexecParams(conn,buf.data,numParams,NULL,values,
19351935
NULL,NULL,0);
19361936
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
1937-
pgfdw_report_error(ERROR,res, true,fsstate->query);
1937+
pgfdw_report_error(ERROR,res,conn,true,fsstate->query);
19381938
PQclear(res);
19391939

19401940
/* Mark the cursor as created, and show no tuples have been retrieved */
@@ -1985,7 +1985,7 @@ fetch_more_data(ForeignScanState *node)
19851985
res=PQexec(conn,sql);
19861986
/* On error, report the original query, not the FETCH. */
19871987
if (PQresultStatus(res)!=PGRES_TUPLES_OK)
1988-
pgfdw_report_error(ERROR,res, false,fsstate->query);
1988+
pgfdw_report_error(ERROR,res,conn,false,fsstate->query);
19891989

19901990
/* Convert the data into HeapTuples */
19911991
numrows=PQntuples(res);
@@ -2091,7 +2091,7 @@ close_cursor(PGconn *conn, unsigned int cursor_number)
20912091
*/
20922092
res=PQexec(conn,sql);
20932093
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
2094-
pgfdw_report_error(ERROR,res, true,sql);
2094+
pgfdw_report_error(ERROR,res,conn,true,sql);
20952095
PQclear(res);
20962096
}
20972097

@@ -2128,7 +2128,7 @@ prepare_foreign_modify(PgFdwModifyState *fmstate)
21282128
NULL);
21292129

21302130
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
2131-
pgfdw_report_error(ERROR,res, true,fmstate->query);
2131+
pgfdw_report_error(ERROR,res,fmstate->conn,true,fmstate->query);
21322132
PQclear(res);
21332133

21342134
/* This action shows that the prepare has been done. */
@@ -2278,7 +2278,7 @@ postgresAnalyzeForeignTable(Relation relation,
22782278
{
22792279
res=PQexec(conn,sql.data);
22802280
if (PQresultStatus(res)!=PGRES_TUPLES_OK)
2281-
pgfdw_report_error(ERROR,res, false,sql.data);
2281+
pgfdw_report_error(ERROR,res,conn,false,sql.data);
22822282

22832283
if (PQntuples(res)!=1||PQnfields(res)!=1)
22842284
elog(ERROR,"unexpected result from deparseAnalyzeSizeSql query");
@@ -2372,7 +2372,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
23722372
{
23732373
res=PQexec(conn,sql.data);
23742374
if (PQresultStatus(res)!=PGRES_COMMAND_OK)
2375-
pgfdw_report_error(ERROR,res, false,sql.data);
2375+
pgfdw_report_error(ERROR,res,conn,false,sql.data);
23762376
PQclear(res);
23772377
res=NULL;
23782378

@@ -2403,7 +2403,7 @@ postgresAcquireSampleRowsFunc(Relation relation, int elevel,
24032403
res=PQexec(conn,fetch_sql);
24042404
/* On error, report the original query, not the FETCH. */
24052405
if (PQresultStatus(res)!=PGRES_TUPLES_OK)
2406-
pgfdw_report_error(ERROR,res, false,sql.data);
2406+
pgfdw_report_error(ERROR,res,conn,false,sql.data);
24072407

24082408
/* Process whatever we got. */
24092409
numrows=PQntuples(res);

‎contrib/postgres_fdw/postgres_fdw.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ extern PGconn *GetConnection(ForeignServer *server, UserMapping *user,
3030
externvoidReleaseConnection(PGconn*conn);
3131
externunsignedintGetCursorNumber(PGconn*conn);
3232
externunsignedintGetPrepStmtNumber(PGconn*conn);
33-
externvoidpgfdw_report_error(intelevel,PGresult*res,boolclear,
34-
constchar*sql);
33+
externvoidpgfdw_report_error(intelevel,PGresult*res,PGconn*conn,
34+
boolclear,constchar*sql);
3535

3636
/* in option.c */
3737
externintExtractConnectionOptions(List*defelems,

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp