Обсуждение: postgres crash on CURSORS
I have found I can crash the backend with the following queries: test=> BEGIN WORK;BEGINtest=> DECLARE bigtable_cursor CURSOR FORtest-> SELECT * FROM bigtable;SELECTtest=> FETCH 3 priorFROM bigtable_cursor;ERROR: parser: parse error at or near "prior"test=> FETCH prior FROM bigtable_cursor;pqReadData()-- backend closed the channel unexpectedly. This probably means the backend terminatedabnormally before or while processing the request.The connection to the server was lost. Attempting reset:Succeeded. Here is the backtrace. Comments? --------------------------------------------------------------------------- #0 0x281d2d65 in kill () #1 0x2821ea5d in abort () #2 0x8146548 in ExcAbort (excP=0x8199294, detail=0, data=0x0, message=0x81812ad "!(MyProc->xmin != 0)") at excabort.c:27 #3 0x8146497 in ExcUnCaught (excP=0x8199294, detail=0, data=0x0, message=0x81812ad "!(MyProc->xmin != 0)") at exc.c:170 #4 0x81464ef in ExcRaise (excP=0x8199294, detail=0, data=0x0, message=0x81812ad "!(MyProc->xmin != 0)") at exc.c:187 #5 0x8145b3a in ExceptionalCondition ( conditionName=0x81812ad "!(MyProc->xmin != 0)", exceptionP=0x8199294, detail=0x0,fileName=0x81811a3 "sinval.c", lineNumber=362) at assert.c:73 #6 0x8103730 in GetSnapshotData (serializable=0 '\000') at sinval.c:362 #7 0x81501d7 in SetQuerySnapshot () at tqual.c:611 #8 0x810c329 in pg_exec_query_dest ( query_string=0x81cd398 "FETCH prior FROM bigtable_cursor;\n", dest=Debug, aclOverride=0)at postgres.c:677 #9 0x810c244 in pg_exec_query ( query_string=0x81cd398 "FETCH prior FROM bigtable_cursor;\n") at postgres.c:607 #10 0x810d43d in PostgresMain (argc=4, argv=0x8047534, real_argc=4, real_argv=0x8047534) at postgres.c:1642 #11 0x80c07fc in main (argc=4, argv=0x8047534) at main.c:103 #12 0x8062a9c in __start () -- Bruce Momjian | http://www.op.net/~candle pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> I have found I can crash the backend with the following queries:
> test=> BEGIN WORK;
> BEGIN
> test=> DECLARE bigtable_cursor CURSOR FOR
test-> SELECT * FROM bigtable;
> SELECT
> test=> FETCH 3 prior FROM bigtable_cursor;
> ERROR: parser: parse error at or near "prior"
> test=> FETCH prior FROM bigtable_cursor;
> pqReadData() -- backend closed the channel unexpectedly.
> This probably means the backend terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Succeeded.
Problem appears to be due to trying to bull ahead with processing after
the current transaction has been aborted by an error. The immediate
cause is these lines in postgres.c:
/* * We have to set query SnapShot in the case of FETCH or COPY TO. */ if
(nodeTag(querytree->utilityStmt)== T_FetchStmt || (nodeTag(querytree->utilityStmt) == T_CopyStmt &&
((CopyStmt *)(querytree->utilityStmt))->direction != FROM)) SetQuerySnapshot();
which are executed without having bothered to check for aborted state.
I think this code should be removed from postgres.c, and the
SetQuerySnapshot call instead made from the Fetch and Copy arms of the
switch statement in ProcessUtility() (utility.c), after doing
CHECK_IF_ABORTED in each case.
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: > > I have found I can crash the backend with the following queries: > > test=> BEGIN WORK; > > BEGIN > > test=> DECLARE bigtable_cursor CURSOR FOR > test-> SELECT * FROM bigtable; > > SELECT > > test=> FETCH 3 prior FROM bigtable_cursor; > > ERROR: parser: parse error at or near "prior" > > test=> FETCH prior FROM bigtable_cursor; > > pqReadData() -- backend closed the channel unexpectedly. > > This probably means the backend terminated abnormally > > before or while processing the request. > > The connection to the server was lost. Attempting reset: Succeeded. > > Problem appears to be due to trying to bull ahead with processing after > the current transaction has been aborted by an error. The immediate > cause is these lines in postgres.c: > > /* > * We have to set query SnapShot in the case of FETCH or COPY TO. > */ > if (nodeTag(querytree->utilityStmt) == T_FetchStmt || > (nodeTag(querytree->utilityStmt) == T_CopyStmt && > ((CopyStmt *)(querytree->utilityStmt))->direction != FROM)) > SetQuerySnapshot(); > > which are executed without having bothered to check for aborted state. > I think this code should be removed from postgres.c, and the > SetQuerySnapshot call instead made from the Fetch and Copy arms of the > switch statement in ProcessUtility() (utility.c), after doing > CHECK_IF_ABORTED in each case. Yes, I saw this code and got totally confused of how to fix it. -- Bruce Momjian | http://www.op.net/~candle pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> -----Original Message----- > From: pgsql-hackers-owner@hub.org [mailto:pgsql-hackers-owner@hub.org]On > Behalf Of Bruce Momjian > > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > > I have found I can crash the backend with the following queries: > > > test=> BEGIN WORK; > > > BEGIN > > > test=> DECLARE bigtable_cursor CURSOR FOR > > test-> SELECT * FROM bigtable; > > > SELECT > > > test=> FETCH 3 prior FROM bigtable_cursor; > > > ERROR: parser: parse error at or near "prior" > > > test=> FETCH prior FROM bigtable_cursor; > > > pqReadData() -- backend closed the channel unexpectedly. > > > This probably means the backend terminated abnormally > > > before or while processing the request. > > > The connection to the server was lost. Attempting reset: Succeeded. > > > > Problem appears to be due to trying to bull ahead with processing after > > the current transaction has been aborted by an error. The immediate > > cause is these lines in postgres.c: > > > > /* > > * We have to set query SnapShot in the case of > FETCH or COPY TO. > > */ > > if (nodeTag(querytree->utilityStmt) == T_FetchStmt || > > (nodeTag(querytree->utilityStmt) == T_CopyStmt && > > ((CopyStmt > *)(querytree->utilityStmt))->direction != FROM)) > > SetQuerySnapshot(); > > > > which are executed without having bothered to check for aborted state. > > I think this code should be removed from postgres.c, and the > > SetQuerySnapshot call instead made from the Fetch and Copy arms of the > > switch statement in ProcessUtility() (utility.c), after doing > > CHECK_IF_ABORTED in each case. > > Yes, I saw this code and got totally confused of how to fix it. > Is it bad to check ABORTED after yyparse() in parser.c ? Regards. Hiroshi Inoue Inoue@tpf.co.jp
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
>>>> which are executed without having bothered to check for aborted state.
>>>> I think this code should be removed from postgres.c, and the
>>>> SetQuerySnapshot call instead made from the Fetch and Copy arms of the
>>>> switch statement in ProcessUtility() (utility.c), after doing
>>>> CHECK_IF_ABORTED in each case.
> Is it bad to check ABORTED after yyparse() in parser.c ?
Yes. Try to execute an END (a/k/a ABORT, ROLLBACK, ...)
The check for abort state has to happen in the appropriate paths of
execution, not in the parser. Not all statements should reject on
abort state.
regards, tom lane
> -----Original Message-----
> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
> Sent: Wednesday, April 05, 2000 1:04 PM
>
> "Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> >>>> which are executed without having bothered to check for
> aborted state.
> >>>> I think this code should be removed from postgres.c, and the
> >>>> SetQuerySnapshot call instead made from the Fetch and Copy
> arms of the
> >>>> switch statement in ProcessUtility() (utility.c), after doing
> >>>> CHECK_IF_ABORTED in each case.
>
> > Is it bad to check ABORTED after yyparse() in parser.c ?
>
> Yes. Try to execute an END (a/k/a ABORT, ROLLBACK, ...)
>
> The check for abort state has to happen in the appropriate paths of
> execution, not in the parser. Not all statements should reject on
> abort state.
>
Are there any statements which should be executable on abort state
except ROLLBACK/COMMIT ?
The following is a sample patch for parser.c.
Regards.
Hiroshi Inoue
Inoue@tpf.co.jp
Index: parser.c
===================================================================
RCS file: /home/cvs/pgcurrent/backend/parser/parser.c,v
retrieving revision 1.2
diff -c -r1.2 parser.c
*** parser.c 2000/01/26 09:58:32 1.2
--- parser.c 2000/04/05 03:54:31
***************
*** 16,21 ****
--- 16,24 ---- #include "parser/analyze.h" #include "parser/gramparse.h" #include "parser/parser.h"
+ #include "nodes/parsenodes.h"
+ #include "access/xact.h"
+ #include "parse.h"
#if defined(FLEX_SCANNER) extern void DeleteBuffer(void);
***************
*** 48,53 ****
--- 51,82 ---- parser_init(typev, nargs);
--- 51,82 ---- parser_init(typev, nargs); yyresult = yyparse();
+ /* To avoid doing processing within an aborted transaction block. */
+ if (!yyresult && IsAbortedTransactionBlockState())
+ {
+ Node *node = lfirst(parsetree);
+
+ if (IsA(node, TransactionStmt))
+ {
+ TransactionStmt *stmt=(TransactionStmt *)node;
+ switch (stmt->command)
+ {
+ case ROLLBACK:
+ case COMMIT:
+ break;
+ default:
+ yyresult = -1;
+ break;
+ }
+ }
+ else
+ yyresult = -1;
+ if (yyresult)
+ {
+ elog(NOTICE, "(transaction already aborted): %s",
+ "queries ignored until END");
+ }
+ } #if defined(FLEX_SCANNER) DeleteBuffer(); #endif /* FLEX_SCANNER */
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
>> The check for abort state has to happen in the appropriate paths of
>> execution, not in the parser. Not all statements should reject on
>> abort state.
> Are there any statements which should be executable on abort state
> except ROLLBACK/COMMIT ?
I dunno ... but offhand, I see no really good reason for checking this
in the parser rather than the way it's done now. Presumably only
utility statements would be candidates for exemption from abort checks,
so checking in the switch statement in ProcessUtility makes sense to
me --- that way the knowledge of the semantics of a given utility
statement is localized.
> The following is a sample patch for parser.c.
The specific patch you propose is definitely inferior to the currently-
committed code, because it does not deal properly with COMMIT/ROLLBACK
appearing within a list of queries. If we are in abort state and
the submitted query string is
SELECT foo ; ROLLBACK ; SELECT bar
it seems to me that the correct response is to reject the first select
and process the second. The currently committed code does so, but
your patch would fail.
regards, tom lane
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > > "Hiroshi Inoue" <Inoue@tpf.co.jp> writes: > >> The check for abort state has to happen in the appropriate paths of > >> execution, not in the parser. Not all statements should reject on > >> abort state. > > > Are there any statements which should be executable on abort state > > except ROLLBACK/COMMIT ? > > I dunno ... but offhand, I see no really good reason for checking this > in the parser rather than the way it's done now. Presumably only > utility statements would be candidates for exemption from abort checks, > so checking in the switch statement in ProcessUtility makes sense to > me --- that way the knowledge of the semantics of a given utility > statement is localized. > Current abort check seems too late. For example,is the following behavior preferable ? =# begin; BEGIN =# aaa; ERROR: parser: parse error at or near "aaa" =# select * from aaaa; ERROR: Relation 'aaaa' does not exist?? existence check ?? Why ?? reindex=# select * from t; -- t is a existent table NOTICE: (transaction aborted): queries ignored until END *ABORT STATE* > > The following is a sample patch for parser.c. > > The specific patch you propose is definitely inferior to the currently- > committed code, because it does not deal properly with COMMIT/ROLLBACK > appearing within a list of queries. If we are in abort state and > the submitted query string is > > SELECT foo ; ROLLBACK ; SELECT bar > > it seems to me that the correct response is to reject the first select > and process the second. The currently committed code does so, but > your patch would fail. > It seems pg_parse_and_plan() returns NIL plan_list and NIL querytree_list in this case. Regards. Hiroshi Inoue Inoue@tpf.co.jp
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
> Current abort check seems too late.
> For example,is the following behavior preferable ?
> =# begin;
> BEGIN
> =# aaa;
> ERROR: parser: parse error at or near "aaa"
> =# select * from aaaa;
> ERROR: Relation 'aaaa' does not exist
> ?? existence check ?? Why ??
> reindex=# select * from t; -- t is a existent table
> NOTICE: (transaction aborted): queries ignored until END
> *ABORT STATE*
Hmm. The error of course appears because we perform parsing and
rewriting before checking for abort. I am not sure whether the rewriter
can introduce begin/commit commands at present, but I would not like to
design the front-end processing on the assumption that it can never do so.
So I think we have to run the query that far before we think about
aborting.
>> If we are in abort state and
>> the submitted query string is
>>
>> SELECT foo ; ROLLBACK ; SELECT bar
>>
>> it seems to me that the correct response is to reject the first select
>> and process the second. The currently committed code does so, but
>> your patch would fail.
> It seems pg_parse_and_plan() returns NIL plan_list and NIL
> querytree_list in this case.
You're not looking at current CVS ;-)
regards, tom lane
> -----Original Message----- > From: Tom Lane [mailto:tgl@sss.pgh.pa.us] > > >> If we are in abort state and > >> the submitted query string is > >> > >> SELECT foo ; ROLLBACK ; SELECT bar > >> > >> it seems to me that the correct response is to reject the first select > >> and process the second. The currently committed code does so, but > >> your patch would fail. > > > It seems pg_parse_and_plan() returns NIL plan_list and NIL > > querytree_list in this case. > > You're not looking at current CVS ;-) > Sorry,I see your change now. Unfortunately I've never used multiple query and understand little about it. For example,how to know using libpq that the first select was ignored ? Regards. Hiroshi Inoue Inoue@tpf.co.jp
"Hiroshi Inoue" <Inoue@tpf.co.jp> writes:
>> From: Tom Lane [mailto:tgl@sss.pgh.pa.us]
>>>>> If we are in abort state and
>>>>> the submitted query string is
>>>>>
>>>>> SELECT foo ; ROLLBACK ; SELECT bar
>>>>>
>>>>> it seems to me that the correct response is to reject the first select
>>>>> and process the second.
> Unfortunately I've never used multiple query and understand
> little about it. For example,how to know using libpq that the first
> select was ignored ?
If you use PQexec then you can't really tell, because you'll only get
back the last command's result. If you use PQsendQuery/PQgetResult
then you'll get back multiple PGresults from a multi-query string, and
you can examine each one to see if it was executed or not.
regards, tom lane