diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c index cc165006f5..53ddc65819 100644 --- a/src/backend/executor/execQual.c +++ b/src/backend/executor/execQual.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.222 2007/09/06 17:31:58 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/execQual.c,v 1.223 2007/10/24 18:37:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -3694,45 +3694,17 @@ ExecEvalArrayCoerceExpr(ArrayCoerceExprState *astate, /* ---------------------------------------------------------------- * ExecEvalCurrentOfExpr * - * Normally, the planner will convert CURRENT OF into a TidScan qualification, - * but we have plain execQual support in case it doesn't. + * The planner must convert CURRENT OF into a TidScan qualification. + * So, we have to be able to do ExecInitExpr on a CurrentOfExpr, + * but we shouldn't ever actually execute it. * ---------------------------------------------------------------- */ static Datum ExecEvalCurrentOfExpr(ExprState *exprstate, ExprContext *econtext, bool *isNull, ExprDoneCond *isDone) { - CurrentOfExpr *cexpr = (CurrentOfExpr *) exprstate->expr; - bool result; - bool lisnull; - Oid tableoid; - ItemPointer tuple_tid; - ItemPointerData cursor_tid; - - if (isDone) - *isDone = ExprSingleResult; - *isNull = false; - - Assert(cexpr->cvarno != INNER); - Assert(cexpr->cvarno != OUTER); - Assert(!TupIsNull(econtext->ecxt_scantuple)); - /* Use slot_getattr to catch any possible mistakes */ - tableoid = DatumGetObjectId(slot_getattr(econtext->ecxt_scantuple, - TableOidAttributeNumber, - &lisnull)); - Assert(!lisnull); - tuple_tid = (ItemPointer) - DatumGetPointer(slot_getattr(econtext->ecxt_scantuple, - SelfItemPointerAttributeNumber, - &lisnull)); - Assert(!lisnull); - - if (execCurrentOf(cexpr, econtext, tableoid, &cursor_tid)) - result = ItemPointerEquals(&cursor_tid, tuple_tid); - else - result = false; - - return BoolGetDatum(result); + elog(ERROR, "CURRENT OF cannot be executed"); + return 0; /* keep compiler quiet */ } diff --git a/src/backend/executor/nodeTidscan.c b/src/backend/executor/nodeTidscan.c index 31a9828b6a..8c217a442b 100644 --- a/src/backend/executor/nodeTidscan.c +++ b/src/backend/executor/nodeTidscan.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.55 2007/06/11 22:22:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/executor/nodeTidscan.c,v 1.56 2007/10/24 18:37:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -68,6 +68,7 @@ TidListCreate(TidScanState *tidstate) tidList = (ItemPointerData *) palloc(numAllocTids * sizeof(ItemPointerData)); numTids = 0; + tidstate->tss_isCurrentOf = false; foreach(l, evalList) { @@ -165,6 +166,7 @@ TidListCreate(TidScanState *tidstate) numAllocTids * sizeof(ItemPointerData)); } tidList[numTids++] = cursor_tid; + tidstate->tss_isCurrentOf = true; } } else @@ -182,6 +184,9 @@ TidListCreate(TidScanState *tidstate) int lastTid; int i; + /* CurrentOfExpr could never appear OR'd with something else */ + Assert(!tidstate->tss_isCurrentOf); + qsort((void *) tidList, numTids, sizeof(ItemPointerData), itemptr_comparator); lastTid = 0; @@ -269,7 +274,8 @@ TidNext(TidScanState *node) /* * XXX shouldn't we check here to make sure tuple matches TID list? In - * runtime-key case this is not certain, is it? + * runtime-key case this is not certain, is it? However, in the + * WHERE CURRENT OF case it might not match anyway ... */ ExecStoreTuple(estate->es_evTuple[scanrelid - 1], @@ -319,6 +325,15 @@ TidNext(TidScanState *node) while (node->tss_TidPtr >= 0 && node->tss_TidPtr < numTids) { tuple->t_self = tidList[node->tss_TidPtr]; + + /* + * For WHERE CURRENT OF, the tuple retrieved from the cursor might + * since have been updated; if so, we should fetch the version that + * is current according to our snapshot. + */ + if (node->tss_isCurrentOf) + heap_get_latest_tid(heapRelation, snapshot, &tuple->t_self); + if (heap_fetch(heapRelation, snapshot, tuple, &buffer, false, NULL)) { /* diff --git a/src/backend/optimizer/path/costsize.c b/src/backend/optimizer/path/costsize.c index d1eb29691a..c722070abc 100644 --- a/src/backend/optimizer/path/costsize.c +++ b/src/backend/optimizer/path/costsize.c @@ -54,7 +54,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/path/costsize.c,v 1.186 2007/09/22 21:36:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/path/costsize.c,v 1.187 2007/10/24 18:37:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -769,6 +769,7 @@ cost_tidscan(Path *path, PlannerInfo *root, { Cost startup_cost = 0; Cost run_cost = 0; + bool isCurrentOf = false; Cost cpu_per_tuple; QualCost tid_qual_cost; int ntuples; @@ -778,9 +779,6 @@ cost_tidscan(Path *path, PlannerInfo *root, Assert(baserel->relid > 0); Assert(baserel->rtekind == RTE_RELATION); - if (!enable_tidscan) - startup_cost += disable_cost; - /* Count how many tuples we expect to retrieve */ ntuples = 0; foreach(l, tidquals) @@ -793,6 +791,12 @@ cost_tidscan(Path *path, PlannerInfo *root, ntuples += estimate_array_length(arraynode); } + else if (IsA(lfirst(l), CurrentOfExpr)) + { + /* CURRENT OF yields 1 tuple */ + isCurrentOf = true; + ntuples++; + } else { /* It's just CTID = something, count 1 tuple */ @@ -800,6 +804,22 @@ cost_tidscan(Path *path, PlannerInfo *root, } } + /* + * We must force TID scan for WHERE CURRENT OF, because only nodeTidscan.c + * understands how to do it correctly. Therefore, honor enable_tidscan + * only when CURRENT OF isn't present. Also note that cost_qual_eval + * counts a CurrentOfExpr as having startup cost disable_cost, which we + * subtract off here; that's to prevent other plan types such as seqscan + * from winning. + */ + if (isCurrentOf) + { + Assert(baserel->baserestrictcost.startup >= disable_cost); + startup_cost -= disable_cost; + } + else if (!enable_tidscan) + startup_cost += disable_cost; + /* * The TID qual expressions will be computed once, any other baserestrict * quals once per retrived tuple. @@ -2002,8 +2022,8 @@ cost_qual_eval_walker(Node *node, cost_qual_eval_context *context) } else if (IsA(node, CurrentOfExpr)) { - /* This is noticeably more expensive than a typical operator */ - context->total.per_tuple += 100 * cpu_operator_cost; + /* Report high cost to prevent selection of anything but TID scan */ + context->total.startup += disable_cost; } else if (IsA(node, SubLink)) { diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h index 82f851eeac..fbb07feae7 100644 --- a/src/include/nodes/execnodes.h +++ b/src/include/nodes/execnodes.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2007, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.178 2007/09/20 17:56:32 tgl Exp $ + * $PostgreSQL: pgsql/src/include/nodes/execnodes.h,v 1.179 2007/10/24 18:37:08 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1087,6 +1087,7 @@ typedef struct BitmapHeapScanState /* ---------------- * TidScanState information * + * isCurrentOf scan has a CurrentOfExpr qual * NumTids number of tids in this scan * TidPtr index of currently fetched tid * TidList evaluated item pointers (array of size NumTids) @@ -1096,6 +1097,7 @@ typedef struct TidScanState { ScanState ss; /* its first field is NodeTag */ List *tss_tidquals; /* list of ExprState nodes */ + bool tss_isCurrentOf; int tss_NumTids; int tss_TidPtr; int tss_MarkTidPtr; diff --git a/src/test/regress/expected/portals.out b/src/test/regress/expected/portals.out index 3638664b1b..b6673073cd 100644 --- a/src/test/regress/expected/portals.out +++ b/src/test/regress/expected/portals.out @@ -982,6 +982,139 @@ SELECT * FROM uctest; 8 | one (2 rows) +-- Check repeated-update and update-then-delete cases +BEGIN; +DECLARE c1 CURSOR FOR SELECT * FROM uctest; +FETCH c1; + f1 | f2 +----+------- + 3 | three +(1 row) + +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; + f1 | f2 +----+------- + 8 | one + 13 | three +(2 rows) + +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; + f1 | f2 +----+------- + 8 | one + 23 | three +(2 rows) + +-- insensitive cursor should not show effects of updates or deletes +FETCH RELATIVE 0 FROM c1; + f1 | f2 +----+------- + 3 | three +(1 row) + +DELETE FROM uctest WHERE CURRENT OF c1; +SELECT * FROM uctest; + f1 | f2 +----+----- + 8 | one +(1 row) + +DELETE FROM uctest WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; + f1 | f2 +----+----- + 8 | one +(1 row) + +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; + f1 | f2 +----+----- + 8 | one +(1 row) + +FETCH RELATIVE 0 FROM c1; + f1 | f2 +----+------- + 3 | three +(1 row) + +ROLLBACK; +SELECT * FROM uctest; + f1 | f2 +----+------- + 3 | three + 8 | one +(2 rows) + +BEGIN; +DECLARE c1 CURSOR FOR SELECT * FROM uctest FOR UPDATE; +FETCH c1; + f1 | f2 +----+------- + 3 | three +(1 row) + +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; + f1 | f2 +----+------- + 8 | one + 13 | three +(2 rows) + +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; + f1 | f2 +----+------- + 8 | one + 23 | three +(2 rows) + +-- sensitive cursor should show effects of updates or deletes +-- XXX current behavior is WRONG +FETCH RELATIVE 0 FROM c1; + f1 | f2 +----+----- + 8 | one +(1 row) + +DELETE FROM uctest WHERE CURRENT OF c1; +SELECT * FROM uctest; + f1 | f2 +----+------- + 23 | three +(1 row) + +DELETE FROM uctest WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; + f1 | f2 +----+------- + 23 | three +(1 row) + +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; + f1 | f2 +----+------- + 23 | three +(1 row) + +FETCH RELATIVE 0 FROM c1; + f1 | f2 +----+---- +(0 rows) + +ROLLBACK; +SELECT * FROM uctest; + f1 | f2 +----+------- + 3 | three + 8 | one +(2 rows) + -- Check inheritance cases CREATE TEMP TABLE ucchild () inherits (uctest); INSERT INTO ucchild values(100, 'hundred'); diff --git a/src/test/regress/sql/portals.sql b/src/test/regress/sql/portals.sql index 382a28c4e3..bdf5956d69 100644 --- a/src/test/regress/sql/portals.sql +++ b/src/test/regress/sql/portals.sql @@ -349,6 +349,46 @@ SELECT * FROM uctest; COMMIT; SELECT * FROM uctest; +-- Check repeated-update and update-then-delete cases +BEGIN; +DECLARE c1 CURSOR FOR SELECT * FROM uctest; +FETCH c1; +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; +-- insensitive cursor should not show effects of updates or deletes +FETCH RELATIVE 0 FROM c1; +DELETE FROM uctest WHERE CURRENT OF c1; +SELECT * FROM uctest; +DELETE FROM uctest WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; +FETCH RELATIVE 0 FROM c1; +ROLLBACK; +SELECT * FROM uctest; + +BEGIN; +DECLARE c1 CURSOR FOR SELECT * FROM uctest FOR UPDATE; +FETCH c1; +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; +SELECT * FROM uctest; +-- sensitive cursor should show effects of updates or deletes +-- XXX current behavior is WRONG +FETCH RELATIVE 0 FROM c1; +DELETE FROM uctest WHERE CURRENT OF c1; +SELECT * FROM uctest; +DELETE FROM uctest WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; +UPDATE uctest SET f1 = f1 + 10 WHERE CURRENT OF c1; -- no-op +SELECT * FROM uctest; +FETCH RELATIVE 0 FROM c1; +ROLLBACK; +SELECT * FROM uctest; + -- Check inheritance cases CREATE TEMP TABLE ucchild () inherits (uctest); INSERT INTO ucchild values(100, 'hundred');