pg_get_indexdef() modification to use TxnSnapshot
I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache(). The latter does not have any old info and thus
provides only the latest info as per the committed txns, which could
result in errors in some scenarios. One such case is mentioned atop
pg_dump.c. The patch is an attempt to fix the same pg_dump's issue.
Any feedback is welcome.
There is a long list of pg_get_* functions which use SearchSysCache()
and thus may expose similar issues. I can give it a try to review the
possibility of converting all of them. Thoughts?
thanks
Shveta
Attachments:
v1-0001-pg_get_indexdef-modification-to-use-TxnSnapshot.patchapplication/octet-stream; name=v1-0001-pg_get_indexdef-modification-to-use-TxnSnapshot.patchDownload
From d97da108bd1bc8d868740553002df65fa43ac565 Mon Sep 17 00:00:00 2001
From: Shveta Malik <shveta.malik@gmail.com>
Date: Fri, 26 May 2023 14:16:50 +0530
Subject: [PATCH v1] pg_get_indexdef modification to use TxnSnapshot.
Change pg_get_indexdef() to use systable_beginscan() based on
transaction-snapshot rather than using SearchSysCache(). The
motivation is to fix the issue mentioned atop pg_dump.c
---
src/backend/utils/adt/ruleutils.c | 168 ++++++++++++++++++++++++++++--
1 file changed, 160 insertions(+), 8 deletions(-)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index d3a973d86b..cd9687084b 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -61,6 +61,7 @@
#include "rewrite/rewriteSupport.h"
#include "utils/array.h"
#include "utils/builtins.h"
+#include "utils/datum.h"
#include "utils/fmgroids.h"
#include "utils/guc.h"
#include "utils/hsearch.h"
@@ -518,6 +519,11 @@ static char *generate_qualified_type_name(Oid typid);
static text *string_to_text(char *str);
static char *flatten_reloptions(Oid relid);
static void get_reloptions(StringInfo buf, Datum reloptions);
+static Datum get_attoptions_using_snapshot(Oid relid, int16 attnum,
+ Snapshot snapshot);
+static char *flatten_reloptions_using_snapshot(Oid relid, Snapshot snapshot);
+
+
#define only_marker(rte) ((rte)->inh ? "" : "ONLY ")
@@ -1267,17 +1273,45 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
StringInfoData buf;
char *str;
char *sep;
+ SysScanDesc scandesc;
+ SysScanDesc scandescRel;
+ SysScanDesc scandescAm;
+ ScanKeyData scankey[1],
+ scankeyRel[1],
+ scankeyAm[1];
+
+ Snapshot snapshot = RegisterSnapshot(GetTransactionSnapshot());
+ Relation indexRelation = table_open(IndexRelationId, AccessShareLock);
+ Relation relation = table_open(RelationRelationId, AccessShareLock);
+ Relation relam = table_open(AccessMethodRelationId, AccessShareLock);
/*
* Fetch the pg_index tuple by the Oid of the index
*/
- ht_idx = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexrelid));
+ ScanKeyInit(&scankey[0], Anum_pg_index_indexrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(indexrelid));
+
+ scandesc = systable_beginscan(indexRelation,
+ IndexRelidIndexId,
+ true, snapshot, 1, scankey);
+
+ ht_idx = systable_getnext(scandesc);
+
if (!HeapTupleIsValid(ht_idx))
{
if (missing_ok)
+ {
+ systable_endscan(scandesc);
+ table_close(indexRelation, AccessShareLock);
+ table_close(relation, AccessShareLock);
+ table_close(relam, AccessShareLock);
+ UnregisterSnapshot(snapshot);
return NULL;
+ }
elog(ERROR, "cache lookup failed for index %u", indexrelid);
}
+
idxrec = (Form_pg_index) GETSTRUCT(ht_idx);
indrelid = idxrec->indrelid;
@@ -1299,7 +1333,15 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
/*
* Fetch the pg_class tuple of the index relation
*/
- ht_idxrel = SearchSysCache1(RELOID, ObjectIdGetDatum(indexrelid));
+ ScanKeyInit(&scankeyRel[0],
+ Anum_pg_class_oid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(indexrelid));
+
+ scandescRel = systable_beginscan(relation, ClassOidIndexId,
+ true, snapshot, 1, scankeyRel);
+
+ ht_idxrel = systable_getnext(scandescRel);
if (!HeapTupleIsValid(ht_idxrel))
elog(ERROR, "cache lookup failed for relation %u", indexrelid);
idxrelrec = (Form_pg_class) GETSTRUCT(ht_idxrel);
@@ -1307,7 +1349,15 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
/*
* Fetch the pg_am tuple of the index' access method
*/
- ht_am = SearchSysCache1(AMOID, ObjectIdGetDatum(idxrelrec->relam));
+ ScanKeyInit(&scankeyAm[0],
+ Anum_pg_am_oid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(idxrelrec->relam));
+
+ scandescAm = systable_beginscan(relam, AmOidIndexId,
+ true, snapshot, 1, scankeyAm);
+
+ ht_am = systable_getnext(scandescAm);
if (!HeapTupleIsValid(ht_am))
elog(ERROR, "cache lookup failed for access method %u",
idxrelrec->relam);
@@ -1432,7 +1482,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
{
int16 opt = indoption->values[keyno];
Oid indcoll = indcollation->values[keyno];
- Datum attoptions = get_attoptions(indexrelid, keyno + 1);
+ Datum attoptions = get_attoptions_using_snapshot(indexrelid, keyno + 1, snapshot);
bool has_options = attoptions != (Datum) 0;
/* Add collation, if not default for column */
@@ -1488,7 +1538,7 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
/*
* If it has options, append "WITH (options)"
*/
- str = flatten_reloptions(indexrelid);
+ str = flatten_reloptions_using_snapshot(indexrelid, snapshot);
if (str)
{
appendStringInfo(&buf, " WITH (%s)", str);
@@ -1539,9 +1589,13 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
}
/* Clean up */
- ReleaseSysCache(ht_idx);
- ReleaseSysCache(ht_idxrel);
- ReleaseSysCache(ht_am);
+ systable_endscan(scandesc);
+ systable_endscan(scandescRel);
+ systable_endscan(scandescAm);
+ table_close(indexRelation, AccessShareLock);
+ table_close(relation, AccessShareLock);
+ table_close(relam, AccessShareLock);
+ UnregisterSnapshot(snapshot);
return buf.data;
}
@@ -12606,3 +12660,101 @@ get_range_partbound_string(List *bound_datums)
return buf->data;
}
+
+/*
+ * get_attoptions_using_snapshot
+ *
+ * Given the relation id and the attribute number,
+ * return the attribute options text[] datum, if any.
+ * It uses snapshot provided by caller to do the scan.
+ */
+static Datum
+get_attoptions_using_snapshot(Oid relid, int16 attnum, Snapshot snapshot)
+{
+ HeapTuple tuple;
+ Datum attopts;
+ Datum result;
+ bool isnull;
+ SysScanDesc scandesc;
+ ScanKeyData scankey[2];
+ Relation relAtt = table_open(AttributeRelationId, AccessShareLock);
+
+ ScanKeyInit(&scankey[0],
+ Anum_pg_attribute_attrelid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(relid));
+
+ ScanKeyInit(&scankey[1],
+ Anum_pg_attribute_attnum,
+ BTEqualStrategyNumber, F_INT2EQ,
+ Int16GetDatum(attnum));
+
+ scandesc = systable_beginscan(relAtt, AttributeRelidNumIndexId,
+ true, snapshot, 2, scankey);
+
+ tuple = systable_getnext(scandesc);
+ if (!HeapTupleIsValid(tuple))
+ {
+ table_close(relAtt, AccessShareLock);
+ elog(ERROR, "cache lookup failed for attribute %d of relation %u",
+ attnum, relid);
+ }
+
+ attopts = SysCacheGetAttr(ATTNAME, tuple, Anum_pg_attribute_attoptions,
+ &isnull);
+
+ if (isnull)
+ result = (Datum) 0;
+ else
+ result = datumCopy(attopts, false, -1); /* text[] */
+
+ table_close(relAtt, AccessShareLock);
+ systable_endscan(scandesc);
+
+ return result;
+}
+
+/*
+ * Generate a C string representing a relation's reloptions, or NULL if none.
+ * It uses snapshot provided by caller to do the scan.
+ */
+static char *
+flatten_reloptions_using_snapshot(Oid relid, Snapshot snapshot)
+{
+ char *result = NULL;
+ HeapTuple tuple;
+ Datum reloptions;
+ bool isnull;
+ SysScanDesc scandesc;
+ ScanKeyData scankey[1];
+ Relation relation = table_open(RelationRelationId, AccessShareLock);
+
+ ScanKeyInit(&scankey[0],
+ Anum_pg_class_oid,
+ BTEqualStrategyNumber, F_OIDEQ,
+ ObjectIdGetDatum(relid));
+
+ scandesc = systable_beginscan(relation, ClassOidIndexId,
+ true, snapshot, 1, scankey);
+
+ tuple = systable_getnext(scandesc);
+ if (!HeapTupleIsValid(tuple))
+ elog(ERROR, "cache lookup failed for relation %u", relid);
+
+ reloptions = SysCacheGetAttr(RELOID, tuple,
+ Anum_pg_class_reloptions, &isnull);
+ if (!isnull)
+ {
+ StringInfoData buf;
+
+ initStringInfo(&buf);
+ get_reloptions(&buf, reloptions);
+
+ result = buf.data;
+ }
+
+ table_close(relation, AccessShareLock);
+ systable_endscan(scandesc);
+
+ return result;
+}
--
2.34.1
On Fri, 26 May 2023 at 15:25, shveta malik <shveta.malik@gmail.com> wrote:
I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache(). The latter does not have any old info and thus
provides only the latest info as per the committed txns, which could
result in errors in some scenarios. One such case is mentioned atop
pg_dump.c. The patch is an attempt to fix the same pg_dump's issue.
Any feedback is welcome.There is a long list of pg_get_* functions which use SearchSysCache()
and thus may expose similar issues. I can give it a try to review the
possibility of converting all of them. Thoughts?
I could reproduce this issue in HEAD with pg_dump dumping a database
having a table and an index like:
create table t1(c1 int);
create index idx1 on t1(c1);
Steps to reproduce:
a) ./pg_dump -d postgres -f dump.txt -- Debug this statement and hold
a breakpoint at getTables function just before it takes lock on the
table t1 b) when the breakpoint is hit, drop the index idx1 c)
Continue the pg_dump execution after dropping the index d) pg_dump
calls pg_get_indexdef to get the index definition e) since
pg_get_indexdef->pg_get_indexdef uses SearchSysCache1 which uses the
latest transaction, it will not get the index information as it sees
the latest catalog snapshot(it will not get the index information). e)
pg_dump will get an empty index statement in this case like:
---------------------------------------------------------------------------------------------
--
-- Name: idx; Type: INDEX; Schema: public; Owner: vignesh
--
;
---------------------------------------------------------------------------------------------
This issue is solved using shveta's patch as it registers the
transaction snapshot and calls systable_beginscan which will not see
the transactions that were started after pg_dump's transaction(the
drop index will not be seen) and gets the index definition as expected
like:
---------------------------------------------------------------------------------------------
--
-- Name: idx; Type: INDEX; Schema: public; Owner: vignesh
--
CREATE INDEX idx ON public.t1 USING btree (c1);
---------------------------------------------------------------------------------------------
Regards,
Vignesh
On Fri, May 26, 2023 at 6:55 PM shveta malik <shveta.malik@gmail.com> wrote:
I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache(). The latter does not have any old info and thus
provides only the latest info as per the committed txns, which could
result in errors in some scenarios. One such case is mentioned atop
pg_dump.c. The patch is an attempt to fix the same pg_dump's issue.
Any feedback is welcome.
Even only in pg_get_indexdef_worker(), there are still many places
where we use syscache lookup: generate_relation_name(),
get_relation_name(), get_attname(), get_atttypetypmodcoll(),
get_opclass_name() etc.
There is a long list of pg_get_* functions which use SearchSysCache()
and thus may expose similar issues. I can give it a try to review the
possibility of converting all of them. Thoughts?
it would be going to be a large refactoring and potentially make the
future implementations such as pg_get_tabledef() etc hard. Have you
considered changes to the SearchSysCache() family so that they
internally use a transaction snapshot that is registered in advance.
Since we are already doing similar things for catalog lookup in
logical decoding, it might be feasible. That way, the changes to
pg_get_XXXdef() functions would be much easier.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
On Tue, 13 Jun 2023 at 11:49, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
On Fri, May 26, 2023 at 6:55 PM shveta malik <shveta.malik@gmail.com> wrote:
I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache(). The latter does not have any old info and thus
provides only the latest info as per the committed txns, which could
result in errors in some scenarios. One such case is mentioned atop
pg_dump.c. The patch is an attempt to fix the same pg_dump's issue.
Any feedback is welcome.Even only in pg_get_indexdef_worker(), there are still many places
where we use syscache lookup: generate_relation_name(),
get_relation_name(), get_attname(), get_atttypetypmodcoll(),
get_opclass_name() etc.There is a long list of pg_get_* functions which use SearchSysCache()
and thus may expose similar issues. I can give it a try to review the
possibility of converting all of them. Thoughts?it would be going to be a large refactoring and potentially make the
future implementations such as pg_get_tabledef() etc hard. Have you
considered changes to the SearchSysCache() family so that they
internally use a transaction snapshot that is registered in advance.
Since we are already doing similar things for catalog lookup in
logical decoding, it might be feasible. That way, the changes to
pg_get_XXXdef() functions would be much easier.
I feel registering an active snapshot before accessing the system
catalog as suggested by Sawada-san will be a better fix to solve the
problem. If this approach is fine, we will have to similarly fix the
other pg_get_*** functions like pg_get_constraintdef,
pg_get_function_arguments, pg_get_function_result,
pg_get_function_identity_arguments, pg_get_function_sqlbody,
pg_get_expr, pg_get_partkeydef, pg_get_statisticsobjdef and
pg_get_triggerdef.
The Attached patch has the changes for the same.
Regards,
Vignesh
Attachments:
0001-pg_get_indexdef-modification-to-access-catalog-based.patchtext/x-patch; charset=US-ASCII; name=0001-pg_get_indexdef-modification-to-access-catalog-based.patchDownload
From c399e185725319e501482e44574eab5830bad0b3 Mon Sep 17 00:00:00 2001
From: Vignesh C <vignesh21@gmail.com>
Date: Wed, 14 Jun 2023 11:35:27 +0530
Subject: [PATCH] pg_get_indexdef modification to access catalog based on the
TxnSnapshot.
Change pg_get_indexdef() to access catalog based on transaction-snapshot by
setting historic snapshot which will prevent accessing a future catalog data
which might occur due to concurrent catalog change. The motivation is to fix
the issue mentioned atop pg_dump.c
---
src/backend/utils/adt/ruleutils.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index d3a973d86b..3f80c77a00 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1154,10 +1154,17 @@ pg_get_indexdef(PG_FUNCTION_ARGS)
prettyFlags = PRETTYFLAG_INDENT;
+ /*
+ * Setup the snapshot to get the catalog information using this snapshot
+ * which will prevent accessing a future catalog data which has occurred
+ * due to concurrent catalog change.
+ */
+ SetupHistoricSnapshot(GetActiveSnapshot(), NULL);
res = pg_get_indexdef_worker(indexrelid, 0, NULL,
false, false,
false, false,
prettyFlags, true);
+ TeardownHistoricSnapshot(false);
if (res == NULL)
PG_RETURN_NULL();
--
2.34.1
Hi hackers,
With '0001-pg_get_indexdef-modification-to-access-catalog-based.patch'
patch,
I confirmed that definition information
can be collected even if the index is droped during pg_dump.
The regression test (make check-world) has passed.
I also tested the view definition for a similar problem.
As per the attached patch and test case,
By using SetupHistoricSnapshot for pg_get_viewdef() as well,
Similarly, definition information can be collected
for VIEW definitions even if the view droped during pg_dump.
The regression test (make check-world) has passed,
and pg_dump's SERIALIZABLE results are improved.
However, in a SERIALIZABLE transaction,
If you actually try to access it, it will cause ERROR,
so it seems to cause confusion.
I think the scope of this improvement should be limited
to the functions listed as pg_get_*** functions at the moment.
---
# test2 pg_get_indexdef,pg_get_viewdef
## create table,index,view
drop table test1;
create table test1(id int);
create index idx1_test1 on test1(id);
create view view1_test1 as select * from test1;
## begin Transaction-A
begin transaction isolation level serializable;
select pg_current_xact_id();
## begin Transaction-B
begin transaction isolation level serializable;
select pg_current_xact_id();
## drop index,view in Transaction-A
drop index idx1_test1;
drop view view1_test1;
commit;
## SELECT pg_get_indexdef,pg_get_viewdef in Transaction-B
SELECT pg_get_indexdef(oid) FROM pg_class WHERE relname = 'idx1_test1';
SELECT pg_get_viewdef(oid) FROM pg_class WHERE relname = 'view1_test1';
## correct info from index and view
postgres=*# SELECT pg_get_indexdef(oid) FROM pg_class WHERE relname =
'idx1_test1';
pg_get_indexdef
----------------------------------------------------------
CREATE INDEX idx1_test1 ON public.test1 USING btree (id)
(1 row)
postgres=*# SELECT pg_get_viewdef(oid) FROM pg_class WHERE relname =
'view1_test1';
pg_get_viewdef
----------------
SELECT id +
FROM test1;
(1 row)
## However, SELECT * FROM view1_test1 cause ERROR because view does not
exist
postgres=*# SELECT * FROM view1_test1;
ERROR: relation "view1_test1" does not exist
LINE 1: SELECT * FROM view1_test1;
Best Regards,
Keisuke Kuroda
NTT COMWARE
Show quoted text
On 2023-06-14 15:31, vignesh C wrote:
On Tue, 13 Jun 2023 at 11:49, Masahiko Sawada <sawada.mshk@gmail.com>
wrote:On Fri, May 26, 2023 at 6:55 PM shveta malik <shveta.malik@gmail.com>
wrote:I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache(). The latter does not have any old info and thus
provides only the latest info as per the committed txns, which could
result in errors in some scenarios. One such case is mentioned atop
pg_dump.c. The patch is an attempt to fix the same pg_dump's issue.
Any feedback is welcome.Even only in pg_get_indexdef_worker(), there are still many places
where we use syscache lookup: generate_relation_name(),
get_relation_name(), get_attname(), get_atttypetypmodcoll(),
get_opclass_name() etc.There is a long list of pg_get_* functions which use SearchSysCache()
and thus may expose similar issues. I can give it a try to review the
possibility of converting all of them. Thoughts?it would be going to be a large refactoring and potentially make the
future implementations such as pg_get_tabledef() etc hard. Have you
considered changes to the SearchSysCache() family so that they
internally use a transaction snapshot that is registered in advance.
Since we are already doing similar things for catalog lookup in
logical decoding, it might be feasible. That way, the changes to
pg_get_XXXdef() functions would be much easier.I feel registering an active snapshot before accessing the system
catalog as suggested by Sawada-san will be a better fix to solve the
problem. If this approach is fine, we will have to similarly fix the
other pg_get_*** functions like pg_get_constraintdef,
pg_get_function_arguments, pg_get_function_result,
pg_get_function_identity_arguments, pg_get_function_sqlbody,
pg_get_expr, pg_get_partkeydef, pg_get_statisticsobjdef and
pg_get_triggerdef.
The Attached patch has the changes for the same.Regards,
Vignesh
Attachments:
pg_get_indexdef-and-pg_get_viewdef.patchtext/x-diff; name=pg_get_indexdef-and-pg_get_viewdef.patchDownload
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 8d5eac4791..c95287104e 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -657,7 +657,14 @@ pg_get_viewdef(PG_FUNCTION_ARGS)
prettyFlags = PRETTYFLAG_INDENT;
+ /*
+ * Setup the snapshot to get the catalog information using this snapshot
+ * which will prevent accessing a future catalog data which has occurred
+ * due to concurrent catalog change.
+ */
+ SetupHistoricSnapshot(GetActiveSnapshot(), NULL);
res = pg_get_viewdef_worker(viewoid, prettyFlags, WRAP_COLUMN_DEFAULT);
+ TeardownHistoricSnapshot(false);
if (res == NULL)
PG_RETURN_NULL();
@@ -1154,10 +1161,17 @@ pg_get_indexdef(PG_FUNCTION_ARGS)
prettyFlags = PRETTYFLAG_INDENT;
+ /*
+ * Setup the snapshot to get the catalog information using this snapshot
+ * which will prevent accessing a future catalog data which has occurred
+ * due to concurrent catalog change.
+ */
+ SetupHistoricSnapshot(GetActiveSnapshot(), NULL);
res = pg_get_indexdef_worker(indexrelid, 0, NULL,
false, false,
false, false,
prettyFlags, true);
+ TeardownHistoricSnapshot(false);
if (res == NULL)
PG_RETURN_NULL();
kuroda.keisuke@nttcom.co.jp writes:
On 2023-06-14 15:31, vignesh C wrote:
I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache().
Has anybody thought about the fact that ALTER TABLE ALTER TYPE
(specifically RememberIndexForRebuilding) absolutely requires seeing
the latest version of the index's definition?
it would be going to be a large refactoring and potentially make the
future implementations such as pg_get_tabledef() etc hard. Have you
considered changes to the SearchSysCache() family so that they
internally use a transaction snapshot that is registered in advance.
A very significant fraction of other SearchSysCache callers likewise
cannot afford to see stale data. We might be able to fix things so
that the SQL-accessible ruleutils functionality works differently, but
we can't just up and change the behavior of cache lookups everywhere.
regards, tom lane
On Thu, Oct 5, 2023 at 11:15 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
kuroda.keisuke@nttcom.co.jp writes:
On 2023-06-14 15:31, vignesh C wrote:
I have attempted to convert pg_get_indexdef() to use
systable_beginscan() based on transaction-snapshot rather than using
SearchSysCache().Has anybody thought about the fact that ALTER TABLE ALTER TYPE
(specifically RememberIndexForRebuilding) absolutely requires seeing
the latest version of the index's definition?it would be going to be a large refactoring and potentially make the
future implementations such as pg_get_tabledef() etc hard. Have you
considered changes to the SearchSysCache() family so that they
internally use a transaction snapshot that is registered in advance.A very significant fraction of other SearchSysCache callers likewise
cannot afford to see stale data. We might be able to fix things so
that the SQL-accessible ruleutils functionality works differently, but
we can't just up and change the behavior of cache lookups everywhere.
This patch was registered in the CommitFest as a bug fix, but I think
it's a much more significant change than that label applies, and it
seems like we have no consensus on what the right design is.
Since there's been no response to these (entirely valid) comments from
Tom in the past 3 months, I've marked this CF entry RwF for now.
--
Robert Haas
EDB: http://www.enterprisedb.com