public inbox for [email protected]
help / color / mirror / Atom feedRe: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM
4+ messages / 4 participants
[nested] [flat]
* Re: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM
@ 2024-05-15 09:14 Alvaro Herrera <[email protected]>
0 siblings, 2 replies; 4+ messages in thread
From: Alvaro Herrera @ 2024-05-15 09:14 UTC (permalink / raw)
To: Bharath Rupireddy <[email protected]>; +Cc: Jeff Davis <[email protected]>; Masahiko Sawada <[email protected]>; pgsql-hackers; Andres Freund <[email protected]>; Dilip Kumar <[email protected]>; Luc Vlaming <[email protected]>; Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Matthias van de Meent <[email protected]>; Alexander Korotkov <[email protected]>
Sorry to interject, but --
On 2024-May-15, Bharath Rupireddy wrote:
> It looks like with the use of the new multi insert table access method
> (TAM) for COPY (v20-0005), pgbench regressed about 35% [1].
Where does this acronym "TAM" comes from for "table access method"? I
find it thoroughly horrible and wish we didn't use it. What's wrong
with using "table AM"? It's not that much longer, much clearer and
reuses our well-established acronym AM.
We don't use IAM anywhere, for example (it's always "index AM"), and I
don't think we'd turn "sequence AM" into SAM either, would we?
--
Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM
@ 2024-05-15 22:03 Andres Freund <[email protected]>
parent: Alvaro Herrera <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Andres Freund @ 2024-05-15 22:03 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Jeff Davis <[email protected]>; Masahiko Sawada <[email protected]>; pgsql-hackers; Dilip Kumar <[email protected]>; Luc Vlaming <[email protected]>; Justin Pryzby <[email protected]>; Michael Paquier <[email protected]>; Matthias van de Meent <[email protected]>; Alexander Korotkov <[email protected]>
Hi,
On 2024-05-15 11:14:14 +0200, Alvaro Herrera wrote:
> On 2024-May-15, Bharath Rupireddy wrote:
>
> > It looks like with the use of the new multi insert table access method
> > (TAM) for COPY (v20-0005), pgbench regressed about 35% [1].
>
> Where does this acronym "TAM" comes from for "table access method"? I
> find it thoroughly horrible and wish we didn't use it. What's wrong
> with using "table AM"? It's not that much longer, much clearer and
> reuses our well-established acronym AM.
Strongly agreed. I don't know why I dislike TAM so much though.
Greetings,
Andres Freund
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM
@ 2024-05-15 23:07 Michael Paquier <[email protected]>
parent: Alvaro Herrera <[email protected]>
1 sibling, 0 replies; 4+ messages in thread
From: Michael Paquier @ 2024-05-15 23:07 UTC (permalink / raw)
To: Alvaro Herrera <[email protected]>; +Cc: Bharath Rupireddy <[email protected]>; Jeff Davis <[email protected]>; Masahiko Sawada <[email protected]>; pgsql-hackers; Andres Freund <[email protected]>; Dilip Kumar <[email protected]>; Luc Vlaming <[email protected]>; Justin Pryzby <[email protected]>; Matthias van de Meent <[email protected]>; Alexander Korotkov <[email protected]>
On Wed, May 15, 2024 at 11:14:14AM +0200, Alvaro Herrera wrote:
> We don't use IAM anywhere, for example (it's always "index AM"), and I
> don't think we'd turn "sequence AM" into SAM either, would we?
SAM is not a term I've seen used for sequence AMs in the past, I don't
intend to use it. TAM is similar strange to me, but perhaps it's just
because I am used to table AMs as a whole.
--
Michael
Attachments:
[application/pgp-signature] signature.asc (833B, ../../[email protected]/2-signature.asc)
download
^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH v45 1/8] Make index_concurrently_create_copy more general
@ 2026-03-24 18:02 Álvaro Herrera <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Álvaro Herrera @ 2026-03-24 18:02 UTC (permalink / raw)
Add a 'boolean concurrent' option, and make it work for both cases.
Also rename it to index_create_copy.
This allows it to be reused for other purposes -- specifically, for
REPACK CONCURRENTLY.
With the CONCURRENTLY option, REPACK cannot simply swap the heap file and
rebuild its indexes. Instead, it needs to build a separate set of indexes
(including system catalog entries) *before* the actual swap, to reduce the
time AccessExclusiveLock needs to be held for. This approach is
different from what CREATE INDEX CONCURRENTLY does.
Per a suggestion from Mihail Nikalayeu.
Author: Antonin Houska <[email protected]>
Discussion: https://postgr.es/m/41104.1754922120@localhost
---
src/backend/catalog/index.c | 39 +++++++++++++++++++-------------
src/backend/commands/indexcmds.c | 15 +++++++-----
src/backend/nodes/makefuncs.c | 9 ++++----
src/include/catalog/index.h | 7 +++---
src/include/nodes/makefuncs.h | 4 +++-
5 files changed, 43 insertions(+), 31 deletions(-)
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index d8219b18c48..de7182a85a9 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -1289,17 +1289,17 @@ index_create(Relation heapRelation,
}
/*
- * index_concurrently_create_copy
+ * index_create_copy
*
- * Create concurrently an index based on the definition of the one provided by
- * caller. The index is inserted into catalogs and needs to be built later
- * on. This is called during concurrent reindex processing.
+ * Create an index based on the definition of the one provided by caller. The
+ * index is inserted into catalogs. If 'concurrently' is TRUE, it needs to be
+ * built later on; otherwise it's built immediately.
*
* "tablespaceOid" is the tablespace to use for this index.
*/
Oid
-index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
- Oid tablespaceOid, const char *newName)
+index_create_copy(Relation heapRelation, bool concurrently,
+ Oid oldIndexId, Oid tablespaceOid, const char *newName)
{
Relation indexRelation;
IndexInfo *oldInfo,
@@ -1318,6 +1318,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
List *indexColNames = NIL;
List *indexExprs = NIL;
List *indexPreds = NIL;
+ int flags = 0;
indexRelation = index_open(oldIndexId, RowExclusiveLock);
@@ -1328,7 +1329,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
* Concurrent build of an index with exclusion constraints is not
* supported.
*/
- if (oldInfo->ii_ExclusionOps != NULL)
+ if (oldInfo->ii_ExclusionOps != NULL && concurrently)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
errmsg("concurrent index creation for exclusion constraints is not supported")));
@@ -1384,9 +1385,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
}
/*
- * Build the index information for the new index. Note that rebuild of
- * indexes with exclusion constraints is not supported, hence there is no
- * need to fill all the ii_Exclusion* fields.
+ * Build the index information for the new index.
*/
newInfo = makeIndexInfo(oldInfo->ii_NumIndexAttrs,
oldInfo->ii_NumIndexKeyAttrs,
@@ -1395,10 +1394,13 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
indexPreds,
oldInfo->ii_Unique,
oldInfo->ii_NullsNotDistinct,
- false, /* not ready for inserts */
- true,
+ !concurrently, /* isready */
+ concurrently, /* concurrent */
indexRelation->rd_indam->amsummarizing,
- oldInfo->ii_WithoutOverlaps);
+ oldInfo->ii_WithoutOverlaps,
+ oldInfo->ii_ExclusionOps,
+ oldInfo->ii_ExclusionProcs,
+ oldInfo->ii_ExclusionStrats);
/*
* Extract the list of column names and the column numbers for the new
@@ -1436,6 +1438,9 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
stattargets[i].isnull = isnull;
}
+ if (concurrently)
+ flags = INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT;
+
/*
* Now create the new index.
*
@@ -1459,7 +1464,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
indcoloptions->values,
stattargets,
reloptionsDatum,
- INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT,
+ flags,
0,
true, /* allow table to be a system catalog? */
false, /* is_internal? */
@@ -2453,7 +2458,8 @@ BuildIndexInfo(Relation index)
indexStruct->indisready,
false,
index->rd_indam->amsummarizing,
- indexStruct->indisexclusion && indexStruct->indisunique);
+ indexStruct->indisexclusion && indexStruct->indisunique,
+ NULL, NULL, NULL);
/* fill in attribute numbers */
for (i = 0; i < numAtts; i++)
@@ -2513,7 +2519,8 @@ BuildDummyIndexInfo(Relation index)
indexStruct->indisready,
false,
index->rd_indam->amsummarizing,
- indexStruct->indisexclusion && indexStruct->indisunique);
+ indexStruct->indisexclusion && indexStruct->indisunique,
+ NULL, NULL, NULL);
/* fill in attribute numbers */
for (i = 0; i < numAtts; i++)
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index dd593ccbc1c..83edab38760 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -244,7 +244,8 @@ CheckIndexCompatible(Oid oldId,
*/
indexInfo = makeIndexInfo(numberOfAttributes, numberOfAttributes,
accessMethodId, NIL, NIL, false, false,
- false, false, amsummarizing, isWithoutOverlaps);
+ false, false, amsummarizing, isWithoutOverlaps,
+ NULL, NULL, NULL);
typeIds = palloc_array(Oid, numberOfAttributes);
collationIds = palloc_array(Oid, numberOfAttributes);
opclassIds = palloc_array(Oid, numberOfAttributes);
@@ -931,7 +932,8 @@ DefineIndex(ParseState *pstate,
!concurrent,
concurrent,
amissummarizing,
- stmt->iswithoutoverlaps);
+ stmt->iswithoutoverlaps,
+ NULL, NULL, NULL);
typeIds = palloc_array(Oid, numberOfAttributes);
collationIds = palloc_array(Oid, numberOfAttributes);
@@ -3989,10 +3991,11 @@ ReindexRelationConcurrently(const ReindexStmt *stmt, Oid relationOid, const Rein
tablespaceid = indexRel->rd_rel->reltablespace;
/* Create new index definition based on given index */
- newIndexId = index_concurrently_create_copy(heapRel,
- idx->indexId,
- tablespaceid,
- concurrentName);
+ newIndexId = index_create_copy(heapRel,
+ true,
+ idx->indexId,
+ tablespaceid,
+ concurrentName);
/*
* Now open the relation of the new index, a session-level lock is
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 3cd35c5c457..8d23aa917e5 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -834,7 +834,8 @@ IndexInfo *
makeIndexInfo(int numattrs, int numkeyattrs, Oid amoid, List *expressions,
List *predicates, bool unique, bool nulls_not_distinct,
bool isready, bool concurrent, bool summarizing,
- bool withoutoverlaps)
+ bool withoutoverlaps, Oid *exclusion_ops, Oid *exclusion_procs,
+ uint16 *exclusion_strats)
{
IndexInfo *n = makeNode(IndexInfo);
@@ -863,9 +864,9 @@ makeIndexInfo(int numattrs, int numkeyattrs, Oid amoid, List *expressions,
n->ii_PredicateState = NULL;
/* exclusion constraints */
- n->ii_ExclusionOps = NULL;
- n->ii_ExclusionProcs = NULL;
- n->ii_ExclusionStrats = NULL;
+ n->ii_ExclusionOps = exclusion_ops;
+ n->ii_ExclusionProcs = exclusion_procs;
+ n->ii_ExclusionStrats = exclusion_strats;
/* speculative inserts */
n->ii_UniqueOps = NULL;
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 36b70689254..56a064ef444 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -101,10 +101,9 @@ extern Oid index_create(Relation heapRelation,
#define INDEX_CONSTR_CREATE_REMOVE_OLD_DEPS (1 << 4)
#define INDEX_CONSTR_CREATE_WITHOUT_OVERLAPS (1 << 5)
-extern Oid index_concurrently_create_copy(Relation heapRelation,
- Oid oldIndexId,
- Oid tablespaceOid,
- const char *newName);
+extern Oid index_create_copy(Relation heapRelation, bool concurrently,
+ Oid oldIndexId, Oid tablespaceOid,
+ const char *newName);
extern void index_concurrently_build(Oid heapRelationId,
Oid indexRelationId);
diff --git a/src/include/nodes/makefuncs.h b/src/include/nodes/makefuncs.h
index bf54d39feb0..40ec249a7a1 100644
--- a/src/include/nodes/makefuncs.h
+++ b/src/include/nodes/makefuncs.h
@@ -99,7 +99,9 @@ extern IndexInfo *makeIndexInfo(int numattrs, int numkeyattrs, Oid amoid,
List *expressions, List *predicates,
bool unique, bool nulls_not_distinct,
bool isready, bool concurrent,
- bool summarizing, bool withoutoverlaps);
+ bool summarizing, bool withoutoverlaps,
+ Oid *exclusion_ops, Oid *exclusion_procs,
+ uint16 *exclusion_strats);
extern Node *makeStringConst(char *str, int location);
extern DefElem *makeDefElem(char *name, Node *arg, int location);
--
2.47.3
--cldir6umqisr673d
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="v45-0002-Do-not-dereference-varattrib_4b-in-VARSIZE_4B.patch"
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-03-24 18:02 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-05-15 09:14 Re: Introduce new multi insert Table AM and improve performance of various SQL commands with it for Heap AM Alvaro Herrera <[email protected]>
2024-05-15 22:03 ` Andres Freund <[email protected]>
2024-05-15 23:07 ` Michael Paquier <[email protected]>
2026-03-24 18:02 [PATCH v45 1/8] Make index_concurrently_create_copy more general Álvaro Herrera <[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