Friday, August 15, 2008

Re: [HACKERS] Patch: plan invalidation vs stored procedures

Index: src/backend/commands/functioncmds.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/functioncmds.c,v
retrieving revision 1.98
diff -c -r1.98 functioncmds.c
*** src/backend/commands/functioncmds.c 18 Jul 2008 03:32:52 -0000 1.98
--- src/backend/commands/functioncmds.c 15 Aug 2008 11:12:51 -0000
***************
*** 59,64 ****
--- 59,65 ----
#include "utils/rel.h"
#include "utils/syscache.h"
#include "utils/tqual.h"
+ #include "utils/inval.h"


static void AlterFunctionOwner_internal(Relation rel, HeapTuple tup,
***************
*** 680,685 ****
--- 681,687 ----
HeapTuple languageTuple;
Form_pg_language languageStruct;
List *as_clause;
+ Oid funcOid;

/* Convert list of names to a name and namespace */
namespaceId = QualifiedNameGetCreationNamespace(stmt->funcname,
***************
*** 817,823 ****
* And now that we have all the parameters, and know we're permitted to do
* so, go ahead and create the function.
*/
! ProcedureCreate(funcname,
namespaceId,
stmt->replace,
returnsSet,
--- 819,825 ----
* And now that we have all the parameters, and know we're permitted to do
* so, go ahead and create the function.
*/
! funcOid = ProcedureCreate(funcname,
namespaceId,
stmt->replace,
returnsSet,
***************
*** 837,842 ****
--- 839,848 ----
PointerGetDatum(proconfig),
procost,
prorows);
+
+ /* Send invalidation on REPLACE */
+ if (stmt->replace)
+ CacheInvalidateProcedure(funcOid);
}


***************
*** 906,911 ****
--- 912,920 ----
object.objectSubId = 0;

performDeletion(&object, stmt->behavior);
+
+ /* Notify that cached plans should be replanned */
+ CacheInvalidateProcedure(funcOid);
}

/*
***************
*** 1029,1034 ****
--- 1038,1046 ----

heap_close(rel, NoLock);
heap_freetuple(tup);
+
+ /* Need plan invalidation after this */
+ CacheInvalidateProcedure(procOid);
}

/*
***************
*** 1294,1299 ****
--- 1306,1314 ----

heap_close(rel, NoLock);
heap_freetuple(tup);
+
+ /* Invalidate plans after this */
+ CacheInvalidateProcedure(funcOid);
}

/*
Index: src/backend/commands/prepare.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/commands/prepare.c,v
retrieving revision 1.89
diff -c -r1.89 prepare.c
*** src/backend/commands/prepare.c 21 Jul 2008 15:26:55 -0000 1.89
--- src/backend/commands/prepare.c 15 Aug 2008 11:12:52 -0000
***************
*** 188,196 ****
/* Shouldn't have a non-fully-planned plancache entry */
if (!entry->plansource->fully_planned)
elog(ERROR, "EXECUTE does not support unplanned prepared statements");
- /* Shouldn't get any non-fixed-result cached plan, either */
- if (!entry->plansource->fixed_result)
- elog(ERROR, "EXECUTE does not support variable-result cached plans");

/* Evaluate parameters, if any */
if (entry->plansource->num_params > 0)
--- 188,193 ----
***************
*** 462,468 ****
cursor_options,
stmt_list,
true,
! true);

/* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries,
--- 459,465 ----
cursor_options,
stmt_list,
true,
! false);

/* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries,
***************
*** 523,533 ****
TupleDesc
FetchPreparedStatementResultDesc(PreparedStatement *stmt)
{
! /*
! * Since we don't allow prepared statements' result tupdescs to change,
! * there's no need for a revalidate call here.
! */
! Assert(stmt->plansource->fixed_result);
if (stmt->plansource->resultDesc)
return CreateTupleDescCopy(stmt->plansource->resultDesc);
else
--- 520,528 ----
TupleDesc
FetchPreparedStatementResultDesc(PreparedStatement *stmt)
{
! /* Revalidate the plan to allow changes in tupdescs. */
! RevalidateCachedPlan(stmt->plansource, false);
!
if (stmt->plansource->resultDesc)
return CreateTupleDescCopy(stmt->plansource->resultDesc);
else
***************
*** 649,657 ****
/* Shouldn't have a non-fully-planned plancache entry */
if (!entry->plansource->fully_planned)
elog(ERROR, "EXPLAIN EXECUTE does not support unplanned prepared statements");
- /* Shouldn't get any non-fixed-result cached plan, either */
- if (!entry->plansource->fixed_result)
- elog(ERROR, "EXPLAIN EXECUTE does not support variable-result cached plans");

/* Replan if needed, and acquire a transient refcount */
cplan = RevalidateCachedPlan(entry->plansource, true);
--- 644,649 ----
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.399
diff -c -r1.399 copyfuncs.c
*** src/backend/nodes/copyfuncs.c 7 Aug 2008 19:35:02 -0000 1.399
--- src/backend/nodes/copyfuncs.c 15 Aug 2008 11:12:53 -0000
***************
*** 84,89 ****
--- 84,90 ----
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(relationOids);
+ COPY_NODE_FIELD(functionOids);
COPY_SCALAR_FIELD(nParamExec);

return newnode;
***************
*** 1866,1871 ****
--- 1867,1873 ----
COPY_NODE_FIELD(limitCount);
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(setOperations);
+ COPY_NODE_FIELD(functionOids);

return newnode;
}
Index: src/backend/optimizer/plan/planner.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/optimizer/plan/planner.c,v
retrieving revision 1.240
diff -c -r1.240 planner.c
*** src/backend/optimizer/plan/planner.c 7 Aug 2008 01:11:50 -0000 1.240
--- src/backend/optimizer/plan/planner.c 15 Aug 2008 11:12:54 -0000
***************
*** 215,220 ****
--- 215,221 ----
result->rowMarks = parse->rowMarks;
result->relationOids = glob->relationOids;
result->nParamExec = list_length(glob->paramlist);
+ result->functionOids = parse->functionOids;

return result;
}
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.376
diff -c -r1.376 analyze.c
*** src/backend/parser/analyze.c 7 Aug 2008 01:11:51 -0000 1.376
--- src/backend/parser/analyze.c 15 Aug 2008 11:12:55 -0000
***************
*** 227,232 ****
--- 227,235 ----
result->querySource = QSRC_ORIGINAL;
result->canSetTag = true;

+ /* Add the function oid list to Query */
+ result->functionOids = pstate->p_functionOids;
+
return result;
}

Index: src/backend/parser/parse_func.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/parse_func.c,v
retrieving revision 1.204
diff -c -r1.204 parse_func.c
*** src/backend/parser/parse_func.c 30 Jul 2008 17:05:04 -0000 1.204
--- src/backend/parser/parse_func.c 15 Aug 2008 11:12:55 -0000
***************
*** 323,328 ****
--- 323,332 ----
parser_errposition(pstate, location)));
}

+ /* add the function Oid to ParseState - this is later copied to Query */
+ if (!list_member_oid(pstate->p_functionOids, funcid))
+ pstate->p_functionOids = lappend_oid(pstate->p_functionOids, funcid);
+
return retval;
}

Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/tcop/postgres.c,v
retrieving revision 1.555
diff -c -r1.555 postgres.c
*** src/backend/tcop/postgres.c 1 Aug 2008 13:16:09 -0000 1.555
--- src/backend/tcop/postgres.c 15 Aug 2008 11:12:57 -0000
***************
*** 2177,2184 ****
errmsg("unnamed prepared statement does not exist")));
}

! /* Prepared statements shouldn't have changeable result descs */
! Assert(psrc->fixed_result);

/*
* If we are in aborted transaction state, we can't run
--- 2177,2184 ----
errmsg("unnamed prepared statement does not exist")));
}

! /* Revalidate the plan to allow tupdesc changes */
! RevalidateCachedPlan(psrc, true);

/*
* If we are in aborted transaction state, we can't run
Index: src/backend/utils/cache/inval.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/cache/inval.c,v
retrieving revision 1.86
diff -c -r1.86 inval.c
*** src/backend/utils/cache/inval.c 19 Jun 2008 21:32:56 -0000 1.86
--- src/backend/utils/cache/inval.c 15 Aug 2008 11:12:57 -0000
***************
*** 115,121 ****
typedef struct InvalidationListHeader
{
InvalidationChunk *cclist; /* list of chunks holding catcache msgs */
! InvalidationChunk *rclist; /* list of chunks holding relcache/smgr msgs */
} InvalidationListHeader;

/*----------------
--- 115,121 ----
typedef struct InvalidationListHeader
{
InvalidationChunk *cclist; /* list of chunks holding catcache msgs */
! InvalidationChunk *rclist; /* list of chunks holding relcache/smgr/proc msgs */
} InvalidationListHeader;

/*----------------
***************
*** 177,182 ****
--- 177,183 ----
#define TWOPHASE_INFO_FILE_AFTER 2 /* relcache file inval */

static void PersistInvalidationMessage(SharedInvalidationMessage *msg);
+ static void CacheRegisterCallback(int cacheid, CacheCallbackFunction func, Datum arg);


/* ----------------------------------------------------------------
***************
*** 363,368 ****
--- 364,393 ----
}

/*
+ * Add a proc inval entry
+ */
+ static void
+ AddProcInvalidationMessage(InvalidationListHeader *hdr,
+ Oid dbId, Oid procId)
+ {
+ SharedInvalidationMessage msg;
+
+ /* Don't add a duplicate item */
+ /* We assume dbId need not be checked because it will never change */
+
+ ProcessMessageList(hdr->rclist,
+ if (msg->pm.id == SHAREDINVALPROC_ID &&
+ msg->pm.procId == procId)
+ return);
+
+ /* OK, add the item */
+ msg.pm.id = SHAREDINVALPROC_ID;
+ msg.pm.dbId = dbId;
+ msg.pm.procId = procId;
+ AddInvalidationMessage(&hdr->rclist, &msg);
+ }
+
+ /*
* Append one list of invalidation messages to another, resetting
* the source list to empty.
*/
***************
*** 465,470 ****
--- 490,512 ----
}

/*
+ * RegisterProcInvalidation
+ *
+ * As above, but register a procedure invalidation event.
+ */
+ static void
+ RegisterProcInvalidation(Oid dbId, Oid procId)
+ {
+ AddProcInvalidationMessage(&transInvalInfo->CurrentCmdInvalidMsgs,
+ dbId, procId);
+
+ /*
+ * As above, just in case there is not an associated catalog change.
+ */
+ (void) GetCurrentCommandId(true);
+ }
+
+ /*
* LocalExecuteInvalidationMessage
*
* Process a single invalidation message (which could be of any type).
***************
*** 516,521 ****
--- 558,577 ----
*/
smgrclosenode(msg->sm.rnode);
}
+ else if (msg->id == SHAREDINVALPROC_ID)
+ {
+ if (msg->rc.dbId == MyDatabaseId)
+ {
+ /* for now we just need to handle callback functions */
+ for (i = 0; i < cache_callback_count; i++)
+ {
+ struct CACHECALLBACK *ccitem = cache_callback_list + i;
+
+ if (ccitem->id == SHAREDINVALPROC_ID)
+ (*ccitem->function) (ccitem->arg, msg->rc.relId);
+ }
+ }
+ }
else
elog(FATAL, "unrecognized SI message id: %d", msg->id);
}
***************
*** 1175,1191 ****
}

/*
! * CacheRegisterSyscacheCallback
* Register the specified function to be called for all future
* invalidation events in the specified cache.
*
- * NOTE: currently, the OID argument to the callback routine is not
- * provided for syscache callbacks; the routine doesn't really get any
- * useful info as to exactly what changed. It should treat every call
- * as a "cache flush" request.
*/
! void
! CacheRegisterSyscacheCallback(int cacheid,
CacheCallbackFunction func,
Datum arg)
{
--- 1231,1254 ----
}

/*
! * CacheInvalidateProcedure
! * Register invalidation of the specified stored procedure.
! *
! */
! void
! CacheInvalidateProcedure(Oid procId)
! {
! RegisterProcInvalidation(MyDatabaseId, procId);
! }
!
! /*
! * CacheRegisterCallback
* Register the specified function to be called for all future
* invalidation events in the specified cache.
*
*/
! static void
! CacheRegisterCallback(int cacheid,
CacheCallbackFunction func,
Datum arg)
{
***************
*** 1199,1204 ****
--- 1262,1286 ----
++cache_callback_count;
}

+
+ /*
+ * CacheRegisterSyscacheCallback
+ * Register the specified function to be called for all future
+ * invalidation events in the specified cache.
+ *
+ * NOTE: currently, the OID argument to the callback routine is not
+ * provided for syscache callbacks; the routine doesn't really get any
+ * useful info as to exactly what changed. It should treat every call
+ * as a "cache flush" request.
+ */
+ void
+ CacheRegisterSyscacheCallback(int cacheid,
+ CacheCallbackFunction func,
+ Datum arg)
+ {
+ CacheRegisterCallback(cacheid, func, arg);
+ }
+
/*
* CacheRegisterRelcacheCallback
* Register the specified function to be called for all future
***************
*** 1212,1223 ****
CacheRegisterRelcacheCallback(CacheCallbackFunction func,
Datum arg)
{
! if (cache_callback_count >= MAX_CACHE_CALLBACKS)
! elog(FATAL, "out of cache_callback_list slots");
!
! cache_callback_list[cache_callback_count].id = SHAREDINVALRELCACHE_ID;
! cache_callback_list[cache_callback_count].function = func;
! cache_callback_list[cache_callback_count].arg = arg;

! ++cache_callback_count;
}
--- 1294,1311 ----
CacheRegisterRelcacheCallback(CacheCallbackFunction func,
Datum arg)
{
! CacheRegisterCallback(SHAREDINVALRELCACHE_ID, func, arg);
! }

! /*
! * CacheRegisterProcCallback
! * Register the specified function to be called for all future
! * proccache invalidation events. The OID of the procedure being
! * invalidated will be passed to the function.
! */
! void
! CacheRegisterProcCallback(CacheCallbackFunction func, Datum arg)
! {
! CacheRegisterCallback(SHAREDINVALPROC_ID, func, arg);
}
+
Index: src/backend/utils/cache/plancache.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/cache/plancache.c,v
retrieving revision 1.19
diff -c -r1.19 plancache.c
*** src/backend/utils/cache/plancache.c 18 Jul 2008 20:26:06 -0000 1.19
--- src/backend/utils/cache/plancache.c 15 Aug 2008 11:12:58 -0000
***************
*** 82,89 ****
--- 82,91 ----
static bool rowmark_member(List *rowMarks, int rt_index);
static bool plan_list_is_transient(List *stmt_list);
static void PlanCacheCallback(Datum arg, Oid relid);
+ static void PlanCacheProcCallback(Datum arg, Oid procId);
static void InvalRelid(Oid relid, LOCKMODE lockmode,
InvalRelidContext *context);
+ static void ScanForInvalidations(Oid objId, bool is_function_oid);


/*
***************
*** 95,100 ****
--- 97,103 ----
InitPlanCache(void)
{
CacheRegisterRelcacheCallback(PlanCacheCallback, (Datum) 0);
+ CacheRegisterProcCallback(PlanCacheProcCallback, (Datum) 0);
}

/*
***************
*** 863,876 ****
}

/*
! * PlanCacheCallback
* Relcache inval callback function
*
! * Invalidate all plans mentioning the given rel, or all plans mentioning
! * any rel at all if relid == InvalidOid.
*/
static void
! PlanCacheCallback(Datum arg, Oid relid)
{
ListCell *lc1;
ListCell *lc2;
--- 866,879 ----
}

/*
! * ScanForInvalidations
* Relcache inval callback function
*
! * Invalidate all plans mentioning the given object, or all plans mentioning
! * any object at all if objId == InvalidOid.
*/
static void
! ScanForInvalidations(Oid objId, bool is_function_oid)
{
ListCell *lc1;
ListCell *lc2;
***************
*** 888,899 ****
foreach(lc2, plan->stmt_list)
{
PlannedStmt *plannedstmt = (PlannedStmt *) lfirst(lc2);

Assert(!IsA(plannedstmt, Query));
if (!IsA(plannedstmt, PlannedStmt))
continue; /* Ignore utility statements */
! if ((relid == InvalidOid) ? plannedstmt->relationOids != NIL :
! list_member_oid(plannedstmt->relationOids, relid))
{
/* Invalidate the plan! */
plan->dead = true;
--- 891,911 ----
foreach(lc2, plan->stmt_list)
{
PlannedStmt *plannedstmt = (PlannedStmt *) lfirst(lc2);
+ bool invalidate = false;

Assert(!IsA(plannedstmt, Query));
if (!IsA(plannedstmt, PlannedStmt))
continue; /* Ignore utility statements */
!
! if (objId == InvalidOid)
! /* invalidate everything */
! invalidate = true;
! else if (is_function_oid)
! invalidate = list_member_oid(plannedstmt->functionOids, objId);
! else
! invalidate = list_member_oid(plannedstmt->relationOids, objId);
!
! if (invalidate)
{
/* Invalidate the plan! */
plan->dead = true;
***************
*** 910,916 ****
*/
InvalRelidContext context;

! context.inval_relid = relid;
context.plan = plan;

foreach(lc2, plan->stmt_list)
--- 922,928 ----
*/
InvalRelidContext context;

! context.inval_relid = objId;
context.plan = plan;

foreach(lc2, plan->stmt_list)
***************
*** 918,930 ****
Query *query = (Query *) lfirst(lc2);

Assert(IsA(query, Query));
! ScanQueryForRelids(query, InvalRelid, (void *) &context);
}
}
}
}

/*
* ResetPlanCache: drop all cached plans.
*/
void
--- 930,977 ----
Query *query = (Query *) lfirst(lc2);

Assert(IsA(query, Query));
!
! if (is_function_oid)
! {
! /* Functions are easy - we already have the list of OIDs in Query */
! if (objId == InvalidOid ||
! list_member_oid(query->functionOids, objId))
! {
! plan->dead = true;
! break;
! }
! }
! else
! {
! /* No precomputed relation OID list in Query - need to scan */
! ScanQueryForRelids(query, InvalRelid, (void *) &context);
! }
}
}
}
}

/*
+ * PlanCacheCallback
+ * Relcache inval callback function
+ */
+ static void
+ PlanCacheCallback(Datum arg, Oid relid)
+ {
+ ScanForInvalidations(relid, false);
+ }
+
+ /*
+ * PlanCacheProcCallback
+ * Stored procedure invalidation callback function
+ */
+ static void
+ PlanCacheProcCallback(Datum arg, Oid procId)
+ {
+ ScanForInvalidations(procId, true);
+ }
+
+ /*
* ResetPlanCache: drop all cached plans.
*/
void
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.371
diff -c -r1.371 parsenodes.h
*** src/include/nodes/parsenodes.h 7 Aug 2008 01:11:51 -0000 1.371
--- src/include/nodes/parsenodes.h 15 Aug 2008 11:12:59 -0000
***************
*** 132,137 ****
--- 132,139 ----

Node *setOperations; /* set-operation tree if this is top level of
* a UNION/INTERSECT/EXCEPT query */
+
+ List *functionOids; /* OIDs of functions the query references */
} Query;


Index: src/include/nodes/plannodes.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/nodes/plannodes.h,v
retrieving revision 1.102
diff -c -r1.102 plannodes.h
*** src/include/nodes/plannodes.h 7 Aug 2008 19:35:02 -0000 1.102
--- src/include/nodes/plannodes.h 15 Aug 2008 11:12:59 -0000
***************
*** 72,77 ****
--- 72,79 ----

List *relationOids; /* OIDs of relations the plan depends on */

+ List *functionOids; /* OIDs of functions the plan depends on */
+
int nParamExec; /* number of PARAM_EXEC Params used */
} PlannedStmt;

Index: src/include/parser/parse_node.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/parser/parse_node.h,v
retrieving revision 1.54
diff -c -r1.54 parse_node.h
*** src/include/parser/parse_node.h 19 Jun 2008 00:46:06 -0000 1.54
--- src/include/parser/parse_node.h 15 Aug 2008 11:12:59 -0000
***************
*** 80,85 ****
--- 80,86 ----
bool p_is_update;
Relation p_target_relation;
RangeTblEntry *p_target_rangetblentry;
+ List *p_functionOids;
} ParseState;

extern ParseState *make_parsestate(ParseState *parentParseState);
Index: src/include/storage/sinval.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/storage/sinval.h,v
retrieving revision 1.48
diff -c -r1.48 sinval.h
*** src/include/storage/sinval.h 19 Jun 2008 21:32:56 -0000 1.48
--- src/include/storage/sinval.h 15 Aug 2008 11:12:59 -0000
***************
*** 74,85 ****
--- 74,95 ----
RelFileNode rnode; /* physical file ID */
} SharedInvalSmgrMsg;

+ #define SHAREDINVALPROC_ID (-3)
+
+ typedef struct
+ {
+ int16 id; /* type field --- must be first */
+ Oid dbId; /* database ID */
+ Oid procId; /* procedure ID */
+ } SharedInvalProcMsg;
+
typedef union
{
int16 id; /* type field --- must be first */
SharedInvalCatcacheMsg cc;
SharedInvalRelcacheMsg rc;
SharedInvalSmgrMsg sm;
+ SharedInvalProcMsg pm;
} SharedInvalidationMessage;


Index: src/include/utils/inval.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/utils/inval.h,v
retrieving revision 1.43
diff -c -r1.43 inval.h
*** src/include/utils/inval.h 19 Jun 2008 00:46:06 -0000 1.43
--- src/include/utils/inval.h 15 Aug 2008 11:12:59 -0000
***************
*** 49,54 ****
--- 49,56 ----

extern void CacheInvalidateRelcacheByRelid(Oid relid);

+ extern void CacheInvalidateProcedure(Oid procId);
+
extern void CacheRegisterSyscacheCallback(int cacheid,
CacheCallbackFunction func,
Datum arg);
***************
*** 56,61 ****
--- 58,66 ----
extern void CacheRegisterRelcacheCallback(CacheCallbackFunction func,
Datum arg);

+ extern void CacheRegisterProcCallback(CacheCallbackFunction func,
+ Datum arg);
+
extern void inval_twophase_postcommit(TransactionId xid, uint16 info,
void *recdata, uint32 len);

Index: src/test/regress/expected/plancache.out
===================================================================
RCS file: /projects/cvsroot/pgsql/src/test/regress/expected/plancache.out,v
retrieving revision 1.7
diff -c -r1.7 plancache.out
*** src/test/regress/expected/plancache.out 8 Jun 2008 22:41:04 -0000 1.7
--- src/test/regress/expected/plancache.out 15 Aug 2008 11:13:01 -0000
***************
*** 53,61 ****
-- since clients probably aren't expecting that to change on the fly
ALTER TABLE pcachetest ADD COLUMN q3 bigint;
EXECUTE prepstmt;
! ERROR: cached plan must not change result type
EXECUTE prepstmt2(123);
! ERROR: cached plan must not change result type
-- but we're nice guys and will let you undo your mistake
ALTER TABLE pcachetest DROP COLUMN q3;
EXECUTE prepstmt;
--- 53,74 ----
-- since clients probably aren't expecting that to change on the fly
ALTER TABLE pcachetest ADD COLUMN q3 bigint;
EXECUTE prepstmt;
! q1 | q2 | q3
! ------------------+-------------------+----
! 4567890123456789 | -4567890123456789 |
! 4567890123456789 | 123 |
! 123 | 456 |
! 123 | 4567890123456789 |
! 4567890123456789 | 4567890123456789 |
! (5 rows)
!
EXECUTE prepstmt2(123);
! q1 | q2 | q3
! -----+------------------+----
! 123 | 456 |
! 123 | 4567890123456789 |
! (2 rows)
!
-- but we're nice guys and will let you undo your mistake
ALTER TABLE pcachetest DROP COLUMN q3;
EXECUTE prepstmt;
Tom Lane wrote:
> Martin Pihlak <martin.pihlak@gmail.com> writes:
>> Changing statement result type is also currently prohibited in
>> StorePreparedStatement. There maybe good reasons for this,
>
> How about "the SQL spec says so"?
>
> Admittedly, it's a bit of a jump from views to prepared statements,
> but the spec is perfectly clear that altering a table doesn't alter
> any views dependent on it: SQL99 11.11 <add column definition> saith

As you said it is a bit of a jump ... For one thing view definitions are
persistent whereas statements are bound to be replanned sooner or later -
reconnects etc. Disallowing replanning after invalidation just postpones
it and meanwhile the cached plans are left unusable ("cached plan must not
change result"). IMHO the problem should be left for the application to handle.
Because this is where it will end up anyway.

Attached is a patch that implements plan invalidation on function DROP,
REPLACE and ALTER. Function oids used by the query are collected in analyze phase
and stored in PlannedStmt. Only plans that reference the altered function are
invalidated. The patch also enables replanning on result set change.

regards,
Martin

No comments: