public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v13 06/18] More refactoring
2+ messages / 2 participants
[nested] [flat]

* [PATCH v13 06/18] More refactoring
@ 2020-11-01 19:46 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Justin Pryzby @ 2020-11-01 19:46 UTC (permalink / raw)

---
 src/backend/commands/indexcmds.c           | 201 ++++++++++-----------
 src/test/regress/expected/create_index.out |   4 +-
 2 files changed, 93 insertions(+), 112 deletions(-)

diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f5fea14ff4..5c5596cd28 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -102,8 +102,8 @@ static void ReindexMultipleInternal(List *relids,
 									ReindexParams *params);
 static bool ReindexRelationConcurrently(Oid relationOid,
 										ReindexParams *params);
-static List *ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
-		ReindexParams *params, MemoryContext private_context);
+static List *ReindexIndexesConcurrently(List *indexIds, ReindexParams *params,
+		MemoryContext private_context);
 static void update_relispartition(Oid relationId, bool newval);
 static inline void set_indexsafe_procflags(void);
 
@@ -2652,8 +2652,8 @@ ReindexIndex(RangeVar *indexRelation, ReindexParams *params, bool isTopLevel)
 			.indexId = indOid,
 			/* other fields set later */
 		};
+
 		ReindexIndexesConcurrently(list_make1(&idxinfo),
-				list_make1_oid(IndexGetRelation(indOid, false)),
 				params, CurrentMemoryContext);
 	}
 	else
@@ -3023,12 +3023,11 @@ reindex_error_callback(void *arg)
 
 
 /*
- * Given a list of index oids, return a list of leaf partitions by removing
- * any intermediate parents.  heaprels is populated with the corresponding
- * tables.
+ * Given a list of index oids, return a new list of leaf partitions by
+ * excluding any intermediate parents.
  */
 static List *
-leaf_indexes(List *inhoids, int options, List **heaprels)
+leaf_indexes(List *inhoids, int options)
 {
 	List		*partitions = NIL;
 	ListCell	*lc;
@@ -3060,7 +3059,6 @@ leaf_indexes(List *inhoids, int options, List **heaprels)
 
 		/* Save partition OID in current MemoryContext */
 		partitions = lappend_oid(partitions, partoid);
-		*heaprels = lappend_oid(*heaprels, tableoid);
 	}
 
 	return partitions;
@@ -3072,13 +3070,11 @@ leaf_indexes(List *inhoids, int options, List **heaprels)
  *
  * Reindex a set of partitions, per the partitioned index or table given
  * by the caller.
- * XXX: should be further refactored with logic from ReindexRelationConcurrently
  */
 static void
 ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 {
-	List	   *partitions = NIL,
-			*heaprels = NIL;
+	List	   *partitions = NIL;
 	char		relkind = get_rel_relkind(relid);
 	char	   *relname = get_rel_name(relid);
 	char	   *relnamespace = get_namespace_name(get_rel_namespace(relid));
@@ -3132,7 +3128,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 	if (relkind == RELKIND_PARTITIONED_INDEX)
 	{
 		old_context = MemoryContextSwitchTo(reindex_context);
-		partitions = leaf_indexes(inhoids, params->options, &heaprels);
+		partitions = leaf_indexes(inhoids, params->options);
 		MemoryContextSwitchTo(old_context);
 	} else {
 		/* Loop over parent tables */
@@ -3145,7 +3141,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 			parttable = table_open(partoid, ShareLock);
 			old_context = MemoryContextSwitchTo(reindex_context);
 			partindexes = RelationGetIndexList(parttable);
-			partindexes = leaf_indexes(partindexes, params->options, &heaprels);
+			partindexes = leaf_indexes(partindexes, params->options);
 			partitions = list_concat(partitions, partindexes);
 
 			MemoryContextSwitchTo(old_context);
@@ -3154,10 +3150,9 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 	}
 
 	if ((params->options & REINDEXOPT_CONCURRENTLY) != 0 &&
-		relkind == RELKIND_PARTITIONED_INDEX &&
 		get_rel_persistence(relid) != RELPERSISTENCE_TEMP)
 	{
-		List			   *idxinfos = NIL;
+		List			    *idxinfos = NIL;
 		ReindexIndexInfo	*idxinfo;
 
 		old_context = MemoryContextSwitchTo(reindex_context);
@@ -3172,7 +3167,7 @@ ReindexPartitions(Oid relid, ReindexParams *params, bool isTopLevel)
 		MemoryContextSwitchTo(old_context);
 
 		/* Process all indexes in a single loop */
-		ReindexIndexesConcurrently(idxinfos, heaprels, params, reindex_context);
+		ReindexIndexesConcurrently(idxinfos, params, reindex_context);
 	} else {
 		/*
 		 * Process each partition listed in a separate transaction.  Note that
@@ -3342,7 +3337,6 @@ ReindexMultipleInternal(List *relids, ReindexParams *params)
 static bool
 ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 {
-	List	   *heapRelationIds = NIL;
 	List	   *indexIds = NIL;
 	List	   *newIndexIds = NIL;
 	ListCell   *lc,
@@ -3395,14 +3389,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 				 */
 				Relation	heapRelation;
 
-				/* Save the list of relation OIDs in private context */
-				oldcontext = MemoryContextSwitchTo(private_context);
-
-				/* Track this relation for session locks */
-				heapRelationIds = lappend_oid(heapRelationIds, relationOid);
-
-				MemoryContextSwitchTo(oldcontext);
-
 				if (IsCatalogRelationOid(relationOid))
 					ereport(ERROR,
 							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -3415,7 +3401,7 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 												  ShareUpdateExclusiveLock);
 					/* leave if relation does not exist */
 					if (!heapRelation)
-						break;
+						break; // XXX: lremove
 				}
 				else
 					heapRelation = table_open(relationOid,
@@ -3473,14 +3459,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 					Relation	toastRelation = table_open(toastOid,
 														   ShareUpdateExclusiveLock);
 
-					/* Save the list of relation OIDs in private context */
-					oldcontext = MemoryContextSwitchTo(private_context);
-
-					/* Track this relation for session locks */
-					heapRelationIds = lappend_oid(heapRelationIds, toastOid);
-
-					MemoryContextSwitchTo(oldcontext);
-
 					foreach(lc2, RelationGetIndexList(toastRelation))
 					{
 						Oid			cellOid = lfirst_oid(lc2);
@@ -3521,78 +3499,6 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 				break;
 			}
 		case RELKIND_INDEX:
-			{
-				Oid			heapId = IndexGetRelation(relationOid,
-													  (params->options & REINDEXOPT_MISSING_OK) != 0);
-				Relation	heapRelation;
-				ReindexIndexInfo *idx;
-
-				/* if relation is missing, leave */
-				if (!OidIsValid(heapId))
-					break;
-
-				if (IsCatalogRelationOid(heapId))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot reindex system catalogs concurrently")));
-
-				/*
-				 * Don't allow reindex for an invalid index on TOAST table, as
-				 * if rebuilt it would not be possible to drop it.  Match
-				 * error message in reindex_index().
-				 */
-				if (IsToastNamespace(get_rel_namespace(relationOid)) &&
-					!get_index_isvalid(relationOid))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot reindex invalid index on TOAST table")));
-
-				/*
-				 * Check if parent relation can be locked and if it exists,
-				 * this needs to be done at this stage as the list of indexes
-				 * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
-				 * should not be used once all the session locks are taken.
-				 */
-				if ((params->options & REINDEXOPT_MISSING_OK) != 0)
-				{
-					heapRelation = try_table_open(heapId,
-												  ShareUpdateExclusiveLock);
-					/* leave if relation does not exist */
-					if (!heapRelation)
-						break;
-				}
-				else
-					heapRelation = table_open(heapId,
-											  ShareUpdateExclusiveLock);
-
-				if (OidIsValid(params->tablespaceOid) &&
-					IsSystemRelation(heapRelation))
-					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot move system relation \"%s\"",
-									get_rel_name(relationOid))));
-
-				table_close(heapRelation, NoLock);
-
-				/* Save the list of relation OIDs in private context */
-				oldcontext = MemoryContextSwitchTo(private_context);
-
-				/* Track the heap relation of this index for session locks */
-				heapRelationIds = list_make1_oid(heapId);
-
-				/*
-				 * Save the list of relation OIDs in private context.  Note
-				 * that invalid indexes are allowed here.
-				 */
-				idx = palloc(sizeof(ReindexIndexInfo));
-				idx->indexId = relationOid;
-				indexIds = lappend(indexIds, idx);
-				/* other fields set later */
-
-				MemoryContextSwitchTo(oldcontext);
-				break;
-			}
-
 		case RELKIND_PARTITIONED_TABLE:
 		case RELKIND_PARTITIONED_INDEX:
 		default:
@@ -3623,10 +3529,10 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
 				 errmsg("cannot move non-shared relation to tablespace \"%s\"",
 						get_tablespace_name(params->tablespaceOid))));
 
-	Assert(heapRelationIds != NIL);
+	// Assert(heapRelationIds != NIL);
 
 	/* Do the work */
-	newIndexIds = ReindexIndexesConcurrently(indexIds, heapRelationIds, params, private_context);
+	newIndexIds = ReindexIndexesConcurrently(indexIds, params, private_context);
 
 	/* Log what we did */
 	if ((params->options & REINDEXOPT_VERBOSE) != 0)
@@ -3668,9 +3574,10 @@ ReindexRelationConcurrently(Oid relationOid, ReindexParams *params)
  * This is called by ReindexRelationConcurrently and
  */
 static List *
-ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
-		ReindexParams *params, MemoryContext private_context)
+ReindexIndexesConcurrently(List *indexIds, ReindexParams *params,
+		MemoryContext private_context)
 {
+	List		*heapRelationIds = NIL;
 	List	   *newIndexIds = NIL;
 	List	   *relationLocks = NIL;
 	List	   *lockTags = NIL;
@@ -3688,6 +3595,80 @@ ReindexIndexesConcurrently(List *indexIds, List *heapRelationIds,
 	};
 	int64		progress_vals[4];
 
+	/* It's not a shared catalog, so refuse to move it to shared tablespace */
+	if (params->tablespaceOid == GLOBALTABLESPACE_OID && false)
+		ereport(ERROR,
+				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+				 errmsg("cannot move non-shared relation to tablespace \"%s\"",
+						get_tablespace_name(params->tablespaceOid))));
+
+	foreach(lc, indexIds)
+	{
+		ReindexIndexInfo	*idx = lfirst(lc);
+		Oid			indexrelid = idx->indexId;
+		Oid			heapId = IndexGetRelation(indexrelid,
+											  (params->options & REINDEXOPT_MISSING_OK) != 0);
+		Relation	heapRelation;
+
+		/* if relation is missing, leave */
+		if (!OidIsValid(heapId))
+			break; // XXX: ldelete?
+
+		if (IsCatalogRelationOid(heapId))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot reindex system catalogs concurrently")));
+
+		/*
+		 * Don't allow reindex for an invalid index on TOAST table, as
+		 * if rebuilt it would not be possible to drop it.  Match
+		 * error message in reindex_index().
+		 */
+		if (IsToastNamespace(get_rel_namespace(indexrelid)) &&
+			!get_index_isvalid(indexrelid))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot reindex invalid index on TOAST table")));
+
+		if (OidIsValid(params->tablespaceOid) &&
+			IsCatalogRelationOid(indexrelid))
+			ereport(ERROR,
+					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+					 errmsg("cannot move system relation \"%s\"",
+							get_rel_name(indexrelid))));
+
+		/*
+		 * Check if parent relation can be locked and if it exists,
+		 * this needs to be done at this stage as the list of indexes
+		 * to rebuild is not complete yet, and REINDEXOPT_MISSING_OK
+		 * should not be used once all the session locks are taken.
+		 */
+		if ((params->options & REINDEXOPT_MISSING_OK) != 0)
+		{
+			heapRelation = try_table_open(heapId,
+										  ShareUpdateExclusiveLock);
+			/* leave if relation does not exist */
+			if (!heapRelation)
+				break; // ldelete
+		}
+		else
+			heapRelation = table_open(heapId,
+									  ShareUpdateExclusiveLock);
+		table_close(heapRelation, NoLock);
+
+		/* Save the list of relation OIDs in private context */
+		oldcontext = MemoryContextSwitchTo(private_context);
+
+		/* Track the heap relation of this index for session locks */
+		heapRelationIds = lappend_oid(heapRelationIds, heapId);
+		// heapRelationIds = list_make1_oid(heapId);
+
+		/* Note that invalid indexes are allowed here. */
+
+		MemoryContextSwitchTo(oldcontext);
+		// break;
+	}
+
 	/*-----
 	 * Now we have all the indexes we want to process in indexIds.
 	 *
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index 6f41adf736..830fdddf24 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2470,12 +2470,12 @@ COMMIT;
 REINDEX TABLE CONCURRENTLY pg_class; -- no catalog relation
 ERROR:  cannot reindex system catalogs concurrently
 REINDEX INDEX CONCURRENTLY pg_class_oid_index; -- no catalog index
-ERROR:  concurrent index creation on system catalog tables is not supported
+ERROR:  cannot reindex system catalogs concurrently
 -- These are the toast table and index of pg_authid.
 REINDEX TABLE CONCURRENTLY pg_toast.pg_toast_1260; -- no catalog toast table
 ERROR:  cannot reindex system catalogs concurrently
 REINDEX INDEX CONCURRENTLY pg_toast.pg_toast_1260_index; -- no catalog toast index
-ERROR:  concurrent index creation on system catalog tables is not supported
+ERROR:  cannot reindex system catalogs concurrently
 REINDEX SYSTEM CONCURRENTLY postgres; -- not allowed for SYSTEM
 ERROR:  cannot reindex system catalogs concurrently
 -- Warns about catalog relations
-- 
2.17.0


--QTprm0S8XgL7H0Dt--





^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: Meson far from ready on Windows
@ 2024-06-18 14:54 Dave Page <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Dave Page @ 2024-06-18 14:54 UTC (permalink / raw)
  To: Andres Freund <[email protected]>; +Cc: PostgreSQL Developers <[email protected]>

Hi

On Tue, 18 Jun 2024 at 15:38, Andres Freund <[email protected]> wrote:

> Hi,
>
> On 2024-06-18 14:53:53 +0100, Dave Page wrote:
> > My next task was to extend that to support PostgreSQL 17 and beyond,
> which
> > is where I started to run into problems. I've attempted builds using
> Meson
> > with each of the dependencies defined in the old-style config.pl, both
> with
> > and without modifying the INCLUDE/LIBS envvars to include the directories
> > for the dependencies (as was found to work in the previous discussion re
> > zlib):
> >
> > Will not successfully configure at all:
>
> Do you have logs for those failures?
>

Sure - https://developer.pgadmin.org/~dpage/build-logs.zip. Those are all
without any modifications to %LIB% or %INCLUDE%.


> I think it's important to note that Meson largely seems to want to use
> > pkgconfig and cmake to find dependencies. pkgconfig isn't really a thing
> on
> > Windows (it is available, but isn't commonly used), and even cmake would
> > typically rely on finding things in either known installation directories
> > or through lib/include vars.
>
> I am not really following what you mean with the cmake bit here?
>
> You can configure additional places to search for cmake files with
> meson setup --cmake-prefix-path=...
>

None of the dependencies include cmake files for distribution on Windows,
so there are no additional files to tell meson to search for. The same
applies to pkgconfig files, which is why the EDB team had to manually craft
them.


>
>
> > There really aren't standard directories like
> > /usr/lib or /usr/include as we find on unixes, or pkgconfig files for
> > everything.
>
> Yes, and?
>

And that's why we really need to be able to locate headers and libraries
easily by passing paths to meson, as we can't rely on pkgconfig, cmake, or
things being in some standard directory on Windows.

Thanks.

-- 
Dave Page
pgAdmin: https://www.pgadmin.org
PostgreSQL: https://www.postgresql.org
EDB: https://www.enterprisedb.com


^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2024-06-18 14:54 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-11-01 19:46 [PATCH v13 06/18] More refactoring Justin Pryzby <[email protected]>
2024-06-18 14:54 Re: Meson far from ready on Windows Dave Page <[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