public inbox for [email protected]
help / color / mirror / Atom feedRe: Create syscaches for pg_extension
6+ messages / 4 participants
[nested] [flat]
* Re: Create syscaches for pg_extension
@ 2024-08-19 06:21 Michael Paquier <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Michael Paquier @ 2024-08-19 06:21 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Pavel Stehule <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Tue, Aug 13, 2024 at 05:38:55PM +0300, Alexander Korotkov wrote:
> +1 from me too
I won't hide that I've wanted that in the past..
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Create syscaches for pg_extension
@ 2024-08-22 01:49 Michael Paquier <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Michael Paquier @ 2024-08-22 01:49 UTC (permalink / raw)
To: Alexander Korotkov <[email protected]>; +Cc: Pavel Stehule <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers
On Mon, Aug 19, 2024 at 03:21:30PM +0900, Michael Paquier wrote:
> I won't hide that I've wanted that in the past..
And I have bumped into a case where this has been helpful today, so
applied. Thanks!
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Create syscaches for pg_extension
@ 2024-09-05 13:41 Andrei Lepikhov <[email protected]>
parent: Michael Paquier <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Andrei Lepikhov @ 2024-09-05 13:41 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: Pavel Stehule <[email protected]>; Jelte Fennema-Nio <[email protected]>; pgsql-hackers; Alexander Korotkov <[email protected]>
On 22/8/2024 03:49, Michael Paquier wrote:
> On Mon, Aug 19, 2024 at 03:21:30PM +0900, Michael Paquier wrote:
>> I won't hide that I've wanted that in the past..
>
> And I have bumped into a case where this has been helpful today, so
> applied. Thanks!
It had been my dream, too, for years. But the reason was the too-costly
call of the get_extension_oid routine (no less than pgbench 2-3% of
overhead when checked it in the planner hook).
It seems that the get_extension_oid routine was not modified when the
sys cache was introduced. What is the reason? It may be that this
routine is redundant now, but if not, and we want to hold the API that
extensions use, maybe we should rewrite it, too.
See the attachment proposing changes.
--
regards, Andrei Lepikhov
From c27241e824de3bf82b1ac7ef263fec13fe8f0ed6 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Thu, 5 Sep 2024 15:03:10 +0200
Subject: [PATCH] Use EXTENSIONNAME syscache to find extension oid.
---
src/backend/commands/extension.c | 28 +++-------------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..e8fbe5aff0 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -64,6 +64,7 @@
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/varlena.h"
@@ -145,32 +146,9 @@ Oid
get_extension_oid(const char *extname, bool missing_ok)
{
Oid result;
- Relation rel;
- SysScanDesc scandesc;
- HeapTuple tuple;
- ScanKeyData entry[1];
-
- rel = table_open(ExtensionRelationId, AccessShareLock);
-
- ScanKeyInit(&entry[0],
- Anum_pg_extension_extname,
- BTEqualStrategyNumber, F_NAMEEQ,
- CStringGetDatum(extname));
-
- scandesc = systable_beginscan(rel, ExtensionNameIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = ((Form_pg_extension) GETSTRUCT(tuple))->oid;
- else
- result = InvalidOid;
-
- systable_endscan(scandesc);
-
- table_close(rel, AccessShareLock);
+ result = GetSysCacheOid1(EXTENSIONNAME, Anum_pg_extension_oid,
+ CStringGetDatum(extname));
if (!OidIsValid(result) && !missing_ok)
ereport(ERROR,
--
2.46.0
Attachments:
[text/plain] 0001-Use-EXTENSIONNAME-syscache-to-find-extension-oid.patch (1.6K, ../../[email protected]/2-0001-Use-EXTENSIONNAME-syscache-to-find-extension-oid.patch)
download | inline diff:
From c27241e824de3bf82b1ac7ef263fec13fe8f0ed6 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Thu, 5 Sep 2024 15:03:10 +0200
Subject: [PATCH] Use EXTENSIONNAME syscache to find extension oid.
---
src/backend/commands/extension.c | 28 +++-------------------------
1 file changed, 3 insertions(+), 25 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..e8fbe5aff0 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -64,6 +64,7 @@
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/varlena.h"
@@ -145,32 +146,9 @@ Oid
get_extension_oid(const char *extname, bool missing_ok)
{
Oid result;
- Relation rel;
- SysScanDesc scandesc;
- HeapTuple tuple;
- ScanKeyData entry[1];
-
- rel = table_open(ExtensionRelationId, AccessShareLock);
-
- ScanKeyInit(&entry[0],
- Anum_pg_extension_extname,
- BTEqualStrategyNumber, F_NAMEEQ,
- CStringGetDatum(extname));
-
- scandesc = systable_beginscan(rel, ExtensionNameIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = ((Form_pg_extension) GETSTRUCT(tuple))->oid;
- else
- result = InvalidOid;
-
- systable_endscan(scandesc);
-
- table_close(rel, AccessShareLock);
+ result = GetSysCacheOid1(EXTENSIONNAME, Anum_pg_extension_oid,
+ CStringGetDatum(extname));
if (!OidIsValid(result) && !missing_ok)
ereport(ERROR,
--
2.46.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Create syscaches for pg_extension
@ 2024-09-05 16:50 Jelte Fennema-Nio <[email protected]>
parent: Andrei Lepikhov <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: Jelte Fennema-Nio @ 2024-09-05 16:50 UTC (permalink / raw)
To: Andrei Lepikhov <[email protected]>; +Cc: Michael Paquier <[email protected]>; Pavel Stehule <[email protected]>; pgsql-hackers; Alexander Korotkov <[email protected]>
On Thu, 5 Sept 2024 at 15:41, Andrei Lepikhov <[email protected]> wrote:
> It seems that the get_extension_oid routine was not modified when the
> sys cache was introduced. What is the reason? It may be that this
> routine is redundant now, but if not, and we want to hold the API that
> extensions use, maybe we should rewrite it, too.
> See the attachment proposing changes.
It seems reasonable to make this function use the new syscache. I
didn't change any existing code in my original patch, because I wanted
to use the syscache APIs directly anyway and I didn't want to make the
patch bigger than strictly necessary. But I totally understand that
for many usages it's probably enough if the existing APIs are simply
faster (on repeated calls). The patch looks fine. But I think
get_extension_name and get_extension_schema should also be updated.
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Create syscaches for pg_extension
@ 2024-09-05 20:03 Andrei Lepikhov <[email protected]>
parent: Jelte Fennema-Nio <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Andrei Lepikhov @ 2024-09-05 20:03 UTC (permalink / raw)
To: Jelte Fennema-Nio <[email protected]>; +Cc: Michael Paquier <[email protected]>; Pavel Stehule <[email protected]>; pgsql-hackers; Alexander Korotkov <[email protected]>
On 5/9/2024 18:50, Jelte Fennema-Nio wrote:
> On Thu, 5 Sept 2024 at 15:41, Andrei Lepikhov <[email protected]> wrote:
>> It seems that the get_extension_oid routine was not modified when the
>> sys cache was introduced. What is the reason? It may be that this
>> routine is redundant now, but if not, and we want to hold the API that
>> extensions use, maybe we should rewrite it, too.
>> See the attachment proposing changes.
>
> It seems reasonable to make this function use the new syscache. I
> didn't change any existing code in my original patch, because I wanted
> to use the syscache APIs directly anyway and I didn't want to make the
> patch bigger than strictly necessary. But I totally understand that
> for many usages it's probably enough if the existing APIs are simply
> faster (on repeated calls). The patch looks fine. But I think
> get_extension_name and get_extension_schema should also be updated.
Thanks, see new patch in attachment.
--
regards, Andrei Lepikhov
From 99b8aec9f6f1bb3bc41454016bb855885c8794b0 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Thu, 5 Sep 2024 15:03:10 +0200
Subject: [PATCH] Lookup an extension data in corresponding syscache.
In addition to the commit 490f869, replace search through the pg_extension
relation with much faster lookups in EXTENSIONOID and EXTENSIONNAME syscaches
whenever possible.
---
src/backend/commands/extension.c | 82 +++++---------------------------
1 file changed, 13 insertions(+), 69 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..7e9f764ee0 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -64,6 +64,7 @@
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/varlena.h"
@@ -145,32 +146,9 @@ Oid
get_extension_oid(const char *extname, bool missing_ok)
{
Oid result;
- Relation rel;
- SysScanDesc scandesc;
- HeapTuple tuple;
- ScanKeyData entry[1];
-
- rel = table_open(ExtensionRelationId, AccessShareLock);
-
- ScanKeyInit(&entry[0],
- Anum_pg_extension_extname,
- BTEqualStrategyNumber, F_NAMEEQ,
- CStringGetDatum(extname));
- scandesc = systable_beginscan(rel, ExtensionNameIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
-
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = ((Form_pg_extension) GETSTRUCT(tuple))->oid;
- else
- result = InvalidOid;
-
- systable_endscan(scandesc);
-
- table_close(rel, AccessShareLock);
+ result = GetSysCacheOid1(EXTENSIONNAME, Anum_pg_extension_oid,
+ CStringGetDatum(extname));
if (!OidIsValid(result) && !missing_ok)
ereport(ERROR,
@@ -190,32 +168,15 @@ char *
get_extension_name(Oid ext_oid)
{
char *result;
- Relation rel;
- SysScanDesc scandesc;
HeapTuple tuple;
- ScanKeyData entry[1];
- rel = table_open(ExtensionRelationId, AccessShareLock);
+ tuple = SearchSysCache1(EXTENSIONOID, ObjectIdGetDatum(ext_oid));
- ScanKeyInit(&entry[0],
- Anum_pg_extension_oid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(ext_oid));
+ if (!HeapTupleIsValid(tuple))
+ return NULL;
- scandesc = systable_beginscan(rel, ExtensionOidIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
-
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = pstrdup(NameStr(((Form_pg_extension) GETSTRUCT(tuple))->extname));
- else
- result = NULL;
-
- systable_endscan(scandesc);
-
- table_close(rel, AccessShareLock);
+ result = pstrdup(NameStr(((Form_pg_extension) GETSTRUCT(tuple))->extname));
+ ReleaseSysCache(tuple);
return result;
}
@@ -229,32 +190,15 @@ Oid
get_extension_schema(Oid ext_oid)
{
Oid result;
- Relation rel;
- SysScanDesc scandesc;
HeapTuple tuple;
- ScanKeyData entry[1];
-
- rel = table_open(ExtensionRelationId, AccessShareLock);
- ScanKeyInit(&entry[0],
- Anum_pg_extension_oid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(ext_oid));
+ tuple = SearchSysCache1(EXTENSIONOID, ObjectIdGetDatum(ext_oid));
- scandesc = systable_beginscan(rel, ExtensionOidIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
-
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = ((Form_pg_extension) GETSTRUCT(tuple))->extnamespace;
- else
- result = InvalidOid;
-
- systable_endscan(scandesc);
+ if (!HeapTupleIsValid(tuple))
+ return InvalidOid;
- table_close(rel, AccessShareLock);
+ result = ((Form_pg_extension) GETSTRUCT(tuple))->extnamespace;
+ ReleaseSysCache(tuple);
return result;
}
--
2.46.0
Attachments:
[text/plain] 0001-Lookup-an-extension-data-in-corresponding-syscache.patch (3.7K, ../../[email protected]/2-0001-Lookup-an-extension-data-in-corresponding-syscache.patch)
download | inline diff:
From 99b8aec9f6f1bb3bc41454016bb855885c8794b0 Mon Sep 17 00:00:00 2001
From: "Andrei V. Lepikhov" <[email protected]>
Date: Thu, 5 Sep 2024 15:03:10 +0200
Subject: [PATCH] Lookup an extension data in corresponding syscache.
In addition to the commit 490f869, replace search through the pg_extension
relation with much faster lookups in EXTENSIONOID and EXTENSIONNAME syscaches
whenever possible.
---
src/backend/commands/extension.c | 82 +++++---------------------------
1 file changed, 13 insertions(+), 69 deletions(-)
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
index 1643c8c69a..7e9f764ee0 100644
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -64,6 +64,7 @@
#include "utils/memutils.h"
#include "utils/rel.h"
#include "utils/snapmgr.h"
+#include "utils/syscache.h"
#include "utils/varlena.h"
@@ -145,32 +146,9 @@ Oid
get_extension_oid(const char *extname, bool missing_ok)
{
Oid result;
- Relation rel;
- SysScanDesc scandesc;
- HeapTuple tuple;
- ScanKeyData entry[1];
-
- rel = table_open(ExtensionRelationId, AccessShareLock);
-
- ScanKeyInit(&entry[0],
- Anum_pg_extension_extname,
- BTEqualStrategyNumber, F_NAMEEQ,
- CStringGetDatum(extname));
- scandesc = systable_beginscan(rel, ExtensionNameIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
-
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = ((Form_pg_extension) GETSTRUCT(tuple))->oid;
- else
- result = InvalidOid;
-
- systable_endscan(scandesc);
-
- table_close(rel, AccessShareLock);
+ result = GetSysCacheOid1(EXTENSIONNAME, Anum_pg_extension_oid,
+ CStringGetDatum(extname));
if (!OidIsValid(result) && !missing_ok)
ereport(ERROR,
@@ -190,32 +168,15 @@ char *
get_extension_name(Oid ext_oid)
{
char *result;
- Relation rel;
- SysScanDesc scandesc;
HeapTuple tuple;
- ScanKeyData entry[1];
- rel = table_open(ExtensionRelationId, AccessShareLock);
+ tuple = SearchSysCache1(EXTENSIONOID, ObjectIdGetDatum(ext_oid));
- ScanKeyInit(&entry[0],
- Anum_pg_extension_oid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(ext_oid));
+ if (!HeapTupleIsValid(tuple))
+ return NULL;
- scandesc = systable_beginscan(rel, ExtensionOidIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
-
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = pstrdup(NameStr(((Form_pg_extension) GETSTRUCT(tuple))->extname));
- else
- result = NULL;
-
- systable_endscan(scandesc);
-
- table_close(rel, AccessShareLock);
+ result = pstrdup(NameStr(((Form_pg_extension) GETSTRUCT(tuple))->extname));
+ ReleaseSysCache(tuple);
return result;
}
@@ -229,32 +190,15 @@ Oid
get_extension_schema(Oid ext_oid)
{
Oid result;
- Relation rel;
- SysScanDesc scandesc;
HeapTuple tuple;
- ScanKeyData entry[1];
-
- rel = table_open(ExtensionRelationId, AccessShareLock);
- ScanKeyInit(&entry[0],
- Anum_pg_extension_oid,
- BTEqualStrategyNumber, F_OIDEQ,
- ObjectIdGetDatum(ext_oid));
+ tuple = SearchSysCache1(EXTENSIONOID, ObjectIdGetDatum(ext_oid));
- scandesc = systable_beginscan(rel, ExtensionOidIndexId, true,
- NULL, 1, entry);
-
- tuple = systable_getnext(scandesc);
-
- /* We assume that there can be at most one matching tuple */
- if (HeapTupleIsValid(tuple))
- result = ((Form_pg_extension) GETSTRUCT(tuple))->extnamespace;
- else
- result = InvalidOid;
-
- systable_endscan(scandesc);
+ if (!HeapTupleIsValid(tuple))
+ return InvalidOid;
- table_close(rel, AccessShareLock);
+ result = ((Form_pg_extension) GETSTRUCT(tuple))->extnamespace;
+ ReleaseSysCache(tuple);
return result;
}
--
2.46.0
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH 2/2] Provide the executor with information on updated columns.
@ 2026-07-01 13:18 Antonin Houska <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Antonin Houska @ 2026-07-01 13:18 UTC (permalink / raw)
When replaying data changes, REPACK calls ExecInsertIndexTuples() for each
INSERT and UPDATE. In the latter case, the function needs to determine the
value of the 'indexUnchanged' hint for the index AM. (The hint is always false
for INSERT.) Therefore, REPACK is supposed to specify which columns are
changed by the UPDATE.
So far, REPACK missed to specify the set of updated columns, so the
'indexUnchanged' hint could incorrectly evaluate to true. Although it should
not affect correctness, it can make the index AM use optimizations that are
not appropriate. Ideally, we should compute the set of updated columns for
each individual UPDATE, however the comparison of the old and new tuple might
add too much overhead.
This patch initializes the set as if all columns were updated. Thus
'indexUnchanged' always evaluates to false. This way the index AM never uses
the related optimizations. It might result in worse structure of the index,
however it seems better to not use the optimizations than to misuse them.
---
src/backend/commands/repack.c | 40 ++++++++++++++++++++++++++++++++++-
1 file changed, 39 insertions(+), 1 deletion(-)
diff --git a/src/backend/commands/repack.c b/src/backend/commands/repack.c
index c9b0c047477..f37bb9a21af 100644
--- a/src/backend/commands/repack.c
+++ b/src/backend/commands/repack.c
@@ -62,6 +62,7 @@
#include "libpq/pqmq.h"
#include "miscadmin.h"
#include "optimizer/optimizer.h"
+#include "parser/parse_relation.h"
#include "pgstat.h"
#include "replication/logicalrelation.h"
#include "storage/bufmgr.h"
@@ -3005,13 +3006,50 @@ static void
initialize_change_context(ChangeContext *chgcxt,
Relation relation, Oid ident_index_id)
{
+ Bitmapset *updatedCols = NULL;
+ RangeTblEntry *rte;
+ List *perminfos = NIL;
+ RTEPermissionInfo *perminfo;
+
chgcxt->cc_rel = relation;
/* Only initialize fields needed by ExecInsertIndexTuples(). */
chgcxt->cc_estate = CreateExecutorState();
+ /*
+ * Initialize updatedCols.
+ *
+ * The point is that ExecInsertIndexTuples() should not pass the
+ * indexUnchanged hint to the index AM unless there's a reason to do
+ * so. For simplicity, we consider all columns updated.
+ *
+ * XXX Should we spend more effort to compare the old and new tuple when
+ * replaying UPDATE, or at least exclude unchanged TOAST values, like we
+ * do in logicalrep_write_tuple()?
+ */
+ for (int i = 0; i < RelationGetDescr(relation)->natts; i++)
+ updatedCols = bms_add_member(updatedCols,
+ i + 1 - FirstLowInvalidHeapAttributeNumber);
+
+ /*
+ * In this case, RTE only needs to have ->perminfoindex initialized, but
+ * there's no reason to not set the fields whose values we have at hand.
+ */
+ rte = makeNode(RangeTblEntry);
+ rte->rtekind = RTE_RELATION;
+ rte->relid = RelationGetRelid(relation);
+ rte->relkind = RelationGetForm(relation)->relkind;
+ /* Create the RTEPermissionInfo instance (and set ->perminfoindex). */
+ addRTEPermissionInfo(&perminfos, rte);
+ /* Make updatedCols available to the executor functions. */
+ perminfo = getRTEPermissionInfo(perminfos, rte);
+ perminfo->updatedCols = updatedCols;
+
+ ExecInitRangeTable(chgcxt->cc_estate, list_make1(rte), perminfos,
+ bms_make_singleton(1));
+
chgcxt->cc_rri = makeNode(ResultRelInfo);
- InitResultRelInfo(chgcxt->cc_rri, relation, 0, NULL, 0);
+ InitResultRelInfo(chgcxt->cc_rri, relation, 1, NULL, 0);
ExecOpenIndices(chgcxt->cc_rri, false);
/*
--
2.52.0
--=-=-=--
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2026-07-01 13:18 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-08-19 06:21 Re: Create syscaches for pg_extension Michael Paquier <[email protected]>
2024-08-22 01:49 ` Michael Paquier <[email protected]>
2024-09-05 13:41 ` Andrei Lepikhov <[email protected]>
2024-09-05 16:50 ` Jelte Fennema-Nio <[email protected]>
2024-09-05 20:03 ` Andrei Lepikhov <[email protected]>
2026-07-01 13:18 [PATCH 2/2] Provide the executor with information on updated columns. Antonin Houska <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox