public inbox for [email protected]
help / color / mirror / Atom feed[PATCH 3/5] pg_rewind: Replace the hybrid list+array data structure with simplehash.
6+ messages / 3 participants
[nested] [flat]
* [PATCH 3/5] pg_rewind: Replace the hybrid list+array data structure with simplehash.
@ 2020-08-19 12:34 Heikki Linnakangas <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Heikki Linnakangas @ 2020-08-19 12:34 UTC (permalink / raw)
Now that simplehash can be use in frontend code, let's make use of it.
---
src/bin/pg_rewind/copy_fetch.c | 4 +-
src/bin/pg_rewind/fetch.c | 2 +-
src/bin/pg_rewind/fetch.h | 2 +-
src/bin/pg_rewind/filemap.c | 284 ++++++++++++++------------------
src/bin/pg_rewind/filemap.h | 67 +++-----
src/bin/pg_rewind/libpq_fetch.c | 4 +-
src/bin/pg_rewind/pg_rewind.c | 12 +-
7 files changed, 168 insertions(+), 207 deletions(-)
diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c
index 18fad32600e..61aed8018b6 100644
--- a/src/bin/pg_rewind/copy_fetch.c
+++ b/src/bin/pg_rewind/copy_fetch.c
@@ -207,9 +207,9 @@ copy_executeFileMap(filemap_t *map)
file_entry_t *entry;
int i;
- for (i = 0; i < map->narray; i++)
+ for (i = 0; i < map->nactions; i++)
{
- entry = map->array[i];
+ entry = map->actions[i];
execute_pagemap(&entry->target_modified_pages, entry->path);
switch (entry->action)
diff --git a/src/bin/pg_rewind/fetch.c b/src/bin/pg_rewind/fetch.c
index f18fe5386ed..f41d0f295ea 100644
--- a/src/bin/pg_rewind/fetch.c
+++ b/src/bin/pg_rewind/fetch.c
@@ -37,7 +37,7 @@ fetchSourceFileList(void)
* Fetch all relation data files that are marked in the given data page map.
*/
void
-executeFileMap(void)
+execute_file_actions(filemap_t *filemap)
{
if (datadir_source)
copy_executeFileMap(filemap);
diff --git a/src/bin/pg_rewind/fetch.h b/src/bin/pg_rewind/fetch.h
index 7cf8b6ea090..b20df8b1537 100644
--- a/src/bin/pg_rewind/fetch.h
+++ b/src/bin/pg_rewind/fetch.h
@@ -25,7 +25,7 @@
*/
extern void fetchSourceFileList(void);
extern char *fetchFile(const char *filename, size_t *filesize);
-extern void executeFileMap(void);
+extern void execute_file_actions(filemap_t *filemap);
/* in libpq_fetch.c */
extern void libpqProcessFileList(void);
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 431e8e760e0..6d48b6076e3 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -3,6 +3,19 @@
* filemap.c
* A data structure for keeping track of files that have changed.
*
+ * This source file contains the logic to decide what to do with different
+ * kinds of files, and the data structure to support it. Before modifying
+ * anything, pg_rewind collects information about all the files and their
+ * attributes in the target and source data directories. It also scans the
+ * WAL log in the target, and collects information about data blocks that
+ * were changed. All this information is stored in a hash table, using the
+ * file path, relative to the root of the data directory, as the key.
+ *
+ * After collecting all the information required, the filemap_finalize()
+ * function scans the hash table and decides what action needs to be taken
+ * for each file. Finally, it sorts the array to the final order that the
+ * actions should be executed in.
+ *
* Copyright (c) 2013-2020, PostgreSQL Global Development Group
*
*-------------------------------------------------------------------------
@@ -14,22 +27,39 @@
#include <unistd.h>
#include "catalog/pg_tablespace_d.h"
+#include "common/hashfn.h"
#include "common/string.h"
#include "datapagemap.h"
#include "filemap.h"
#include "pg_rewind.h"
#include "storage/fd.h"
-filemap_t *filemap = NULL;
+/*
+ * Define a hash table which we can use to store information about the files
+ * mentioned in the backup manifest.
+ */
+static uint32 hash_string_pointer(const char *s);
+#define SH_PREFIX filehash
+#define SH_ELEMENT_TYPE file_entry_t
+#define SH_KEY_TYPE const char *
+#define SH_KEY path
+#define SH_HASH_KEY(tb, key) hash_string_pointer(key)
+#define SH_EQUAL(tb, a, b) (strcmp(a, b) == 0)
+#define SH_SCOPE static inline
+#define SH_RAW_ALLOCATOR pg_malloc0
+#define SH_DECLARE
+#define SH_DEFINE
+#include "lib/simplehash.h"
+
+static filehash_hash *filehash;
static bool isRelDataFile(const char *path);
static char *datasegpath(RelFileNode rnode, ForkNumber forknum,
BlockNumber segno);
-static int path_cmp(const void *a, const void *b);
-static file_entry_t *get_filemap_entry(const char *path, bool create);
+static file_entry_t *insert_filehash_entry(const char *path);
+static file_entry_t *lookup_filehash_entry(const char *path);
static int final_filemap_cmp(const void *a, const void *b);
-static void filemap_list_to_array(filemap_t *map);
static bool check_file_excluded(const char *path, bool is_source);
/*
@@ -131,54 +161,26 @@ static const struct exclude_list_item excludeFiles[] =
};
/*
- * Create a new file map (stored in the global pointer "filemap").
+ * Initialize the hash table for the file map.
*/
void
-filemap_create(void)
+filemap_init(void)
{
- filemap_t *map;
-
- map = pg_malloc(sizeof(filemap_t));
- map->first = map->last = NULL;
- map->nlist = 0;
- map->array = NULL;
- map->narray = 0;
-
- Assert(filemap == NULL);
- filemap = map;
+ filehash = filehash_create(1000, NULL);
}
-/* Look up or create entry for 'path' */
+/* Look up entry for 'path', creating new one if it doesn't exists */
static file_entry_t *
-get_filemap_entry(const char *path, bool create)
+insert_filehash_entry(const char *path)
{
- filemap_t *map = filemap;
file_entry_t *entry;
- file_entry_t **e;
- file_entry_t key;
- file_entry_t *key_ptr;
+ bool found;
- if (map->array)
+ entry = filehash_insert(filehash, path, &found);
+ if (!found)
{
- key.path = (char *) path;
- key_ptr = &key;
- e = bsearch(&key_ptr, map->array, map->narray, sizeof(file_entry_t *),
- path_cmp);
- }
- else
- e = NULL;
-
- if (e)
- entry = *e;
- else if (!create)
- entry = NULL;
- else
- {
- /* Create a new entry for this file */
- entry = pg_malloc(sizeof(file_entry_t));
entry->path = pg_strdup(path);
entry->isrelfile = isRelDataFile(path);
- entry->action = FILE_ACTION_UNDECIDED;
entry->target_exists = false;
entry->target_type = FILE_TYPE_UNDEFINED;
@@ -192,21 +194,18 @@ get_filemap_entry(const char *path, bool create)
entry->source_size = 0;
entry->source_link_target = NULL;
- entry->next = NULL;
-
- if (map->last)
- {
- map->last->next = entry;
- map->last = entry;
- }
- else
- map->first = map->last = entry;
- map->nlist++;
+ entry->action = FILE_ACTION_UNDECIDED;
}
return entry;
}
+static file_entry_t *
+lookup_filehash_entry(const char *path)
+{
+ return filehash_lookup(filehash, path);
+}
+
/*
* Callback for processing source file list.
*
@@ -220,8 +219,6 @@ process_source_file(const char *path, file_type_t type, size_t size,
{
file_entry_t *entry;
- Assert(filemap->array == NULL);
-
/*
* Skip the control file. It is handled specially, after copying all the
* other files.
@@ -262,7 +259,9 @@ process_source_file(const char *path, file_type_t type, size_t size,
pg_fatal("data file \"%s\" in source is not a regular file", path);
/* Remember this source file */
- entry = get_filemap_entry(path, true);
+ entry = insert_filehash_entry(path);
+ if (entry->source_exists)
+ pg_fatal("duplicate source file \"%s\"", path);
entry->source_exists = true;
entry->source_type = type;
entry->source_size = size;
@@ -280,7 +279,6 @@ void
process_target_file(const char *path, file_type_t type, size_t size,
const char *link_target)
{
- filemap_t *map = filemap;
file_entry_t *entry;
/*
@@ -295,21 +293,6 @@ process_target_file(const char *path, file_type_t type, size_t size,
* from the target data folder all paths which have been filtered out from
* the source data folder when processing the source files.
*/
- if (map->array == NULL)
- {
- /* on first call, initialize lookup array */
- if (map->nlist == 0)
- {
- /* should not happen */
- pg_fatal("source file list is empty");
- }
-
- filemap_list_to_array(map);
-
- Assert(map->array != NULL);
-
- qsort(map->array, map->narray, sizeof(file_entry_t *), path_cmp);
- }
/*
* Like in process_source_file, pretend that pg_wal is always a directory.
@@ -318,7 +301,9 @@ process_target_file(const char *path, file_type_t type, size_t size,
type = FILE_TYPE_DIRECTORY;
/* Remember this target file */
- entry = get_filemap_entry(path, true);
+ entry = insert_filehash_entry(path);
+ if (entry->target_exists)
+ pg_fatal("duplicate source file \"%s\"", path);
entry->target_exists = true;
entry->target_type = type;
entry->target_size = size;
@@ -331,7 +316,7 @@ process_target_file(const char *path, file_type_t type, size_t size,
* changed blocks in the pagemap of the file.
*
* NOTE: All the files on both systems must have already been added to the
- * file map!
+ * hash table!
*/
void
process_target_wal_block_change(ForkNumber forknum, RelFileNode rnode,
@@ -342,47 +327,45 @@ process_target_wal_block_change(ForkNumber forknum, RelFileNode rnode,
BlockNumber blkno_inseg;
int segno;
- Assert(filemap->array);
-
segno = blkno / RELSEG_SIZE;
blkno_inseg = blkno % RELSEG_SIZE;
path = datasegpath(rnode, forknum, segno);
- entry = get_filemap_entry(path, false);
+ entry = lookup_filehash_entry(path);
pfree(path);
+ /*
+ * If the block still exists in both systems, remember it. Otherwise we
+ * can safely ignore it.
+ *
+ * If the block is beyond the EOF in the source system, or the file doesn't
+ * doesn'exist in the source at all, we're going to truncate/remove it away
+ * from the target anyway. Likewise, if it doesn't exist in the target
+ * anymore, we will copy it over with the "tail" from the source system,
+ * anyway.
+ *
+ * It is possible to find WAL for a file that doesn't exist on either
+ * system anymore. It means that the relation was dropped later in the
+ * target system, and independently on the source system too, or that
+ * it was created and dropped in the target system and it never existed
+ * in the source. Either way, we can safely ignore it.
+ */
if (entry)
{
- int64 end_offset;
-
Assert(entry->isrelfile);
if (entry->target_type != FILE_TYPE_REGULAR)
pg_fatal("unexpected page modification for directory or symbolic link \"%s\"",
entry->path);
- /*
- * If the block beyond the EOF in the source system, no need to
- * remember it now, because we're going to truncate it away from the
- * target anyway. Also no need to remember the block if it's beyond
- * the current EOF in the target system; we will copy it over with the
- * "tail" from the source system, anyway.
- */
- end_offset = (blkno_inseg + 1) * BLCKSZ;
- if (end_offset <= entry->source_size &&
- end_offset <= entry->target_size)
- datapagemap_add(&entry->target_modified_pages, blkno_inseg);
- }
- else
- {
- /*
- * If we don't have any record of this file in the file map, it means
- * that it's a relation that doesn't exist in the source system. It
- * could exist in the target system; we haven't moved the target-only
- * entries from the linked list to the array yet! But in any case, if
- * it doesn't exist in the source it will be removed from the target
- * too, and we can safely ignore it.
- */
+ if (entry->target_exists && entry->source_exists)
+ {
+ off_t end_offset;
+
+ end_offset = (blkno_inseg + 1) * BLCKSZ;
+ if (end_offset <= entry->source_size && end_offset <= entry->target_size)
+ datapagemap_add(&entry->target_modified_pages, blkno_inseg);
+ }
}
}
@@ -444,34 +427,6 @@ check_file_excluded(const char *path, bool is_source)
return false;
}
-/*
- * Convert the linked list of entries in map->first/last to the array,
- * map->array.
- */
-static void
-filemap_list_to_array(filemap_t *map)
-{
- int narray;
- file_entry_t *entry,
- *next;
-
- map->array = (file_entry_t **)
- pg_realloc(map->array,
- (map->nlist + map->narray) * sizeof(file_entry_t *));
-
- narray = map->narray;
- for (entry = map->first; entry != NULL; entry = next)
- {
- map->array[narray++] = entry;
- next = entry->next;
- entry->next = NULL;
- }
- Assert(narray == map->nlist + map->narray);
- map->narray = narray;
- map->nlist = 0;
- map->first = map->last = NULL;
-}
-
static const char *
action_to_str(file_action_t action)
{
@@ -499,32 +454,31 @@ action_to_str(file_action_t action)
* Calculate the totals needed for progress reports.
*/
void
-calculate_totals(void)
+calculate_totals(filemap_t *filemap)
{
file_entry_t *entry;
int i;
- filemap_t *map = filemap;
- map->total_size = 0;
- map->fetch_size = 0;
+ filemap->total_size = 0;
+ filemap->fetch_size = 0;
- for (i = 0; i < map->narray; i++)
+ for (i = 0; i < filemap->nactions; i++)
{
- entry = map->array[i];
+ entry = filemap->actions[i];
if (entry->source_type != FILE_TYPE_REGULAR)
continue;
- map->total_size += entry->source_size;
+ filemap->total_size += entry->source_size;
if (entry->action == FILE_ACTION_COPY)
{
- map->fetch_size += entry->source_size;
+ filemap->fetch_size += entry->source_size;
continue;
}
if (entry->action == FILE_ACTION_COPY_TAIL)
- map->fetch_size += (entry->source_size - entry->target_size);
+ filemap->fetch_size += (entry->source_size - entry->target_size);
if (entry->target_modified_pages.bitmapsize > 0)
{
@@ -533,7 +487,7 @@ calculate_totals(void)
iter = datapagemap_iterate(&entry->target_modified_pages);
while (datapagemap_next(iter, &blk))
- map->fetch_size += BLCKSZ;
+ filemap->fetch_size += BLCKSZ;
pg_free(iter);
}
@@ -541,15 +495,14 @@ calculate_totals(void)
}
void
-print_filemap(void)
+print_filemap(filemap_t *filemap)
{
- filemap_t *map = filemap;
file_entry_t *entry;
int i;
- for (i = 0; i < map->narray; i++)
+ for (i = 0; i < filemap->nactions; i++)
{
- entry = map->array[i];
+ entry = filemap->actions[i];
if (entry->action != FILE_ACTION_NONE ||
entry->target_modified_pages.bitmapsize > 0)
{
@@ -671,15 +624,6 @@ datasegpath(RelFileNode rnode, ForkNumber forknum, BlockNumber segno)
return path;
}
-static int
-path_cmp(const void *a, const void *b)
-{
- file_entry_t *fa = *((file_entry_t **) a);
- file_entry_t *fb = *((file_entry_t **) b);
-
- return strcmp(fa->path, fb->path);
-}
-
/*
* In the final stage, the filemap is sorted so that removals come last.
* From disk space usage point of view, it would be better to do removals
@@ -710,16 +654,17 @@ final_filemap_cmp(const void *a, const void *b)
/*
* Decide what to do with each file.
*/
-void
+filemap_t *
filemap_finalize()
{
int i;
+ filehash_iterator it;
+ file_entry_t *entry;
+ filemap_t *filemap;
- filemap_list_to_array(filemap);
-
- for (i = 0; i < filemap->narray; i++)
+ filehash_start_iterate(filehash, &it);
+ while ((entry = filehash_iterate(filehash, &it)) != NULL)
{
- file_entry_t *entry = filemap->array[i];
file_action_t action;
if (!entry->target_exists && entry->source_exists)
@@ -825,7 +770,34 @@ filemap_finalize()
entry->action = action;
}
- /* Sort the actions to the order that they should be performed */
- qsort(filemap->array, filemap->narray, sizeof(file_entry_t *),
+ /*
+ * Turn the hash table into an array, sorted in the order that the actions
+ * should be performed.
+ */
+ filemap = pg_malloc(offsetof(filemap_t, actions) +
+ filehash->members * sizeof(file_entry_t *));
+ filemap->nactions = filehash->members;
+ filehash_start_iterate(filehash, &it);
+ i = 0;
+ while ((entry = filehash_iterate(filehash, &it)) != NULL)
+ {
+ filemap->actions[i++] = entry;
+ }
+
+ qsort(&filemap->actions, filemap->nactions, sizeof(file_entry_t *),
final_filemap_cmp);
+
+ return filemap;
+}
+
+
+/*
+ * Helper function for filemap hash table.
+ */
+static uint32
+hash_string_pointer(const char *s)
+{
+ unsigned char *ss = (unsigned char *) s;
+
+ return hash_bytes(ss, strlen(s));
}
diff --git a/src/bin/pg_rewind/filemap.h b/src/bin/pg_rewind/filemap.h
index a5e8df57f40..3660ffe0990 100644
--- a/src/bin/pg_rewind/filemap.h
+++ b/src/bin/pg_rewind/filemap.h
@@ -12,15 +12,6 @@
#include "storage/block.h"
#include "storage/relfilenode.h"
-/*
- * For every file found in the local or remote system, we have a file entry
- * that contains information about the file on both systems. For relation
- * files, there is also a page map that marks pages in the file that were
- * changed in the target after the last common checkpoint. Each entry also
- * contains an 'action' field, which says what we are going to do with the
- * file.
- */
-
/* these enum values are sorted in the order we want actions to be processed */
typedef enum
{
@@ -44,9 +35,21 @@ typedef enum
FILE_TYPE_SYMLINK
} file_type_t;
+/*
+ * For every file found in the local or remote system, we have a file entry
+ * that contains information about the file on both systems. For relation
+ * files, there is also a page map that marks pages in the file that were
+ * changed in the target after the last common checkpoint.
+ *
+ * When gathering information, these are kept in a hash table, private to
+ * filemap.c. filemap_finalize() fills in the 'action' field, sorts all the
+ * entries, and returns them in an array, ready for executing the actions.
+ */
typedef struct file_entry_t
{
- char *path;
+ uint32 status; /* hash status */
+
+ const char *path;
bool isrelfile; /* is it a relation data file? */
/*
@@ -71,44 +74,25 @@ typedef struct file_entry_t
* What will we do to the file?
*/
file_action_t action;
-
- struct file_entry_t *next;
} file_entry_t;
+/*
+ * This represents the final decisions on what to do with each file.
+ * 'actions' array contains an entry for each file, sorted in the order
+ * that their actions should executed.
+ */
typedef struct filemap_t
{
- /*
- * New entries are accumulated to a linked list, in process_source_file
- * and process_target_file.
- */
- file_entry_t *first;
- file_entry_t *last;
- int nlist; /* number of entries currently in list */
-
- /*
- * After processing all the remote files, the entries in the linked list
- * are moved to this array. After processing local files, too, all the
- * local entries are added to the array by filemap_finalize, and sorted in
- * the final order. After filemap_finalize, all the entries are in the
- * array, and the linked list is empty.
- */
- file_entry_t **array;
- int narray; /* current length of array */
-
- /*
- * Summary information.
- */
+ /* Summary information, filled by calculate_totals() */
uint64 total_size; /* total size of the source cluster */
uint64 fetch_size; /* number of bytes that needs to be copied */
-} filemap_t;
-extern filemap_t *filemap;
-
-extern void filemap_create(void);
-extern void calculate_totals(void);
-extern void print_filemap(void);
+ int nactions; /* size of 'actions' array */
+ file_entry_t *actions[FLEXIBLE_ARRAY_MEMBER];
+} filemap_t;
/* Functions for populating the filemap */
+extern void filemap_init(void);
extern void process_source_file(const char *path, file_type_t type,
size_t size, const char *link_target);
extern void process_target_file(const char *path, file_type_t type,
@@ -116,6 +100,9 @@ extern void process_target_file(const char *path, file_type_t type,
extern void process_target_wal_block_change(ForkNumber forknum,
RelFileNode rnode,
BlockNumber blkno);
-extern void filemap_finalize(void);
+
+extern filemap_t *filemap_finalize(void);
+extern void calculate_totals(filemap_t *filemap);
+extern void print_filemap(filemap_t *filemap);
#endif /* FILEMAP_H */
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 7fc9161b8c8..9c541bb73d5 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -460,9 +460,9 @@ libpq_executeFileMap(filemap_t *map)
PQresultErrorMessage(res));
PQclear(res);
- for (i = 0; i < map->narray; i++)
+ for (i = 0; i < map->nactions; i++)
{
- entry = map->array[i];
+ entry = map->actions[i];
/* If this is a relation file, copy the modified blocks */
execute_pagemap(&entry->target_modified_pages, entry->path);
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 210984d302b..2bdeed26c53 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -129,6 +129,7 @@ main(int argc, char **argv)
TimeLineID endtli;
ControlFileData ControlFile_new;
bool writerecoveryconf = false;
+ filemap_t *filemap;
pg_logging_init(argv[0]);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_rewind"));
@@ -371,10 +372,11 @@ main(int argc, char **argv)
/*
* Collect information about all files in the target and source systems.
*/
- filemap_create();
if (showprogress)
pg_log_info("reading source file list");
+ filemap_init();
fetchSourceFileList();
+
if (showprogress)
pg_log_info("reading target file list");
traverse_datadir(datadir_target, &process_target_file);
@@ -395,13 +397,13 @@ main(int argc, char **argv)
* We have collected all information we need from both systems. Decide
* what to do with each file.
*/
- filemap_finalize();
+ filemap = filemap_finalize();
if (showprogress)
- calculate_totals();
+ calculate_totals(filemap);
/* this is too verbose even for verbose mode */
if (debug)
- print_filemap();
+ print_filemap(filemap);
/*
* Ok, we're ready to start copying things over.
@@ -421,7 +423,7 @@ main(int argc, char **argv)
* modified the target directory and there is no turning back!
*/
- executeFileMap();
+ execute_file_actions(filemap);
progress_report(true);
--
2.20.1
--------------D93EDEBFB124D563B723F4BD
Content-Type: text/x-patch; charset=UTF-8;
name="0004-pg_rewind-Refactor-the-abstraction-to-fetch-from-loc.patch"
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename*0="0004-pg_rewind-Refactor-the-abstraction-to-fetch-from-loc.pa";
filename*1="tch"
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: Proposal to Enable/Disable Index using ALTER INDEX
@ 2024-09-10 21:35 David Rowley <[email protected]>
2024-09-22 17:42 ` [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
0 siblings, 1 reply; 6+ messages in thread
From: David Rowley @ 2024-09-10 21:35 UTC (permalink / raw)
To: Nathan Bossart <[email protected]>; +Cc: Shayon Mukherjee <[email protected]>; [email protected]
On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <[email protected]> wrote:
>
> On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote:
> > If we get the skip scan feature for PG18, then there's likely going to
> > be lots of people with indexes that they might want to consider
> > removing after upgrading. Maybe this is a good time to consider this
> > feature as it possibly won't ever be more useful than it will be after
> > we get skip scans.
>
> +1, this is something I've wanted for some time. There was some past
> discussion, too [0].
>
> [0] https://postgr.es/m/flat/ed8c9ed7-bb5d-aaec-065b-ad4893645deb%402ndQuadrant.com
Thanks for digging that up. I'd forgotten about that. I see there was
pushback from having this last time, which is now over 6 years ago.
In the meantime, we still have nothing to make this easy for people.
I think the most important point I read in that thread is [1]. Maybe
what I mentioned in [2] is a good workaround.
Additionally, I think there will need to be syntax in CREATE INDEX for
this. Without that pg_get_indexdef() might return SQL that does not
reflect the current state of the index. MySQL seems to use "CREATE
INDEX name ON table (col) [VISIBLE|INVISIBLE]".
David
[1] https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm%40alap3.anarazel.de
[2] https://www.postgresql.org/message-id/CAKJS1f_L7y_BTGESp5Qd6BSRHXP0mj3x9O9C_U27GU478UwpBw%40mail.gma...
^ permalink raw reply [nested|flat] 6+ messages in thread
* [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX
2024-09-10 21:35 Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
@ 2024-09-22 17:42 ` Shayon Mukherjee <[email protected]>
2024-09-22 18:20 ` Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
2024-09-22 22:44 ` Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
0 siblings, 2 replies; 6+ messages in thread
From: Shayon Mukherjee @ 2024-09-22 17:42 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected]
Hello,
Thank you for all the feedback and insights. Work was busy, so I didn't get
to follow up earlier.
This patch introduces the ability to enable or disable indexes using ALTER
INDEX
and CREATE INDEX commands.
Original motivation for the problem and proposal for a patch
can be found here[0]
This patch contains the relevant implementation details, new regression
tests and documentation.
It passes all the existing specs and the newly added regression tests. It
compiles, so the
patch can be applied for testing as well.
I have attached the patch in this email, and have also shared it on my
Github fork[1]. Mostly so
that I can ensure the full CI passes.
*Implementation details:*
- New Grammar:
* ALTER INDEX ... ENABLE/DISABLE
* CREATE INDEX ... DISABLE
- Default state is enabled. Indexes are only disabled when explicitly
instructed via CREATE INDEX ... DISABLE or ALTER INDEX ... DISABLE.
- Primary Key and Unique constraint indexes are always enabled as well. The
ENABLE/DISABLE grammar is not supported for these types of indexes. They
can
be later disabled via ALTER INDEX ... ENABLE/DISABLE.
- ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the
pg_index
catalog to protect against indcheckxmin.
- pg_get_indexdef() support for the new functionality and grammar. This
change is
reflected in \d output for tables and pg_dump. We show the DISABLE syntax
accordingly.
- Updated create_index.sql regression test to cover the new grammar and
verify
that disabled indexes are not used in queries.
- Modified get_index_paths() and build_index_paths() to exclude disabled
indexes from consideration during query planning.
- No changes are made to stop the index from getting rebuilt. This way we
ensure no
data miss or corruption when index is re-enabled.
- TOAST indexes are supported and enabled by default.
- REINDEX CONCURRENTLY is supported as well and the existing state of
pg_index.indisenabled
is carried over accordingly.
- catversion.h is updated with a new CATALOG_VERSION_NO to reflect change
in pg_index
schema.
- See the changes in create_index.sql to get an idea of the grammar and sql
statements.
- See the changes in create_index.out to get an idea of the catalogue
states and EXPLAIN
output to see when an index is getting used or isn't (when disabled).
I am looking forward to any and all feedback on this patch, including but
not limited to
code quality, tests, and fundamental logic.
Thank you for the reviews and feedback.
[0]
https://www.postgresql.org/message-id/CANqtF-oXKe0M%3D0QOih6H%2BsZRjE2BWAbkW_1%2B9nMEAMLxUJg5jA%40ma...
[1] https://github.com/shayonj/postgres/pull/1
Best,
Shayon
On Tue, Sep 10, 2024 at 5:35 PM David Rowley <[email protected]> wrote:
> On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <[email protected]>
> wrote:
> >
> > On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote:
> > > If we get the skip scan feature for PG18, then there's likely going to
> > > be lots of people with indexes that they might want to consider
> > > removing after upgrading. Maybe this is a good time to consider this
> > > feature as it possibly won't ever be more useful than it will be after
> > > we get skip scans.
> >
> > +1, this is something I've wanted for some time. There was some past
> > discussion, too [0].
> >
> > [0]
> https://postgr.es/m/flat/ed8c9ed7-bb5d-aaec-065b-ad4893645deb%402ndQuadrant.com
>
> Thanks for digging that up. I'd forgotten about that. I see there was
> pushback from having this last time, which is now over 6 years ago.
> In the meantime, we still have nothing to make this easy for people.
>
> I think the most important point I read in that thread is [1]. Maybe
> what I mentioned in [2] is a good workaround.
>
> Additionally, I think there will need to be syntax in CREATE INDEX for
> this. Without that pg_get_indexdef() might return SQL that does not
> reflect the current state of the index. MySQL seems to use "CREATE
> INDEX name ON table (col) [VISIBLE|INVISIBLE]".
>
> David
>
> [1]
> https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm%40alap3.anarazel.de
> [2]
> https://www.postgresql.org/message-id/CAKJS1f_L7y_BTGESp5Qd6BSRHXP0mj3x9O9C_U27GU478UwpBw%40mail.gma...
>
--
Kind Regards,
Shayon Mukherjee
Attachments:
[application/octet-stream] 0001-Introduce-the-ability-to-enable-disable-indexes.patch (47.3K, ../../CANqtF-oBaBtRfw9O7GAoHN3nNEZQYsW3oaGfD+wJfG8R29nZYw@mail.gmail.com/3-0001-Introduce-the-ability-to-enable-disable-indexes.patch)
download | inline diff:
From 24c3439250d99d59650922e623ff23591dd4286e Mon Sep 17 00:00:00 2001
From: Shayon Mukherjee <[email protected]>
Date: Sun, 22 Sep 2024 11:59:45 -0400
Subject: [PATCH] Introduce the ability to enable/disable indexes
This patch introduces the ability to enable or disable indexes using ALTER INDEX
and CREATE INDEX commands.
Original motivation for the problem and proposal for a patch
can be found here: https://www.postgresql.org/message-id/CANqtF-oXKe0M%3D0QOih6H%2BsZRjE2BWAbkW_1%2B9nMEAMLxUJg5jA%40mail.gmail.com
This patch passes all the existing specs and the newly added regression tests. The patch
is ready for review and test. It compiles, so the can patch can be applied for testing as well.
Implementation details:
- New Grammar:
* ALTER INDEX ... ENABLE/DISABLE
* CREATE INDEX ... DISABLE
- Default state is enabled. Indexes are only disabled when explicitly
instructed via CREATE INDEX ... DISABLE or ALTER INDEX ... DISABLE.
- Primary Key and Unique constraint indexes are always enabled as well. The
ENABLE/DISABLE grammar is not supported for these types of indexes. They can
be later disabled via ALTER INDEX ... ENABLE/DISABLE.
- ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the pg_index
catalog to protect against indcheckxmin.
- pg_get_indexdef() support the new functionality and grammar. This change is
reflected in \d output for tables and pg_dump. We show the DISABLE syntax accordingly.
- Updated create_index.sql regression test to cover the new grammar and verify
that disabled indexes are not used in queries.
- Modified get_index_paths() and build_index_paths() to exclude disabled
indexes from consideration during query planning.
- No changes are made to stop the index from getting rebuilt. This way ensure no
data miss or corruption when index is re-nabled.
- TOAST indexes are supported and enabled by default.
- REINDEX CONCURRENTLY is supported as well and existing state of pg_index.indisenabled
is carried over accordingly.
- catversion.h is updated with a new CATALOG_VERSION_NO to reflect change in pg_index
schema.
- See the changes in create_index.sql to get an idea of the grammar and sql statements.
- See the changes in create_index.out to get an idea of the catalogue states and EXPLAIN
output to see when an index is getting used or isn't (when disabled).
I am looking forward to any and all feedback on this patch, including but not limited to
code quality, tests, fundamental logic.
---
doc/src/sgml/catalogs.sgml | 11 +
doc/src/sgml/ref/alter_index.sgml | 39 ++++
doc/src/sgml/ref/create_index.sgml | 30 +++
src/backend/bootstrap/bootparse.y | 2 +
src/backend/catalog/index.c | 31 ++-
src/backend/catalog/toasting.c | 2 +-
src/backend/commands/indexcmds.c | 4 +
src/backend/commands/tablecmds.c | 94 +++++++-
src/backend/optimizer/path/indxpath.c | 12 +
src/backend/optimizer/util/plancat.c | 2 +
src/backend/parser/gram.y | 48 +++-
src/backend/parser/parse_utilcmd.c | 3 +
src/backend/utils/adt/ruleutils.c | 4 +
src/backend/utils/cache/relcache.c | 1 +
src/include/catalog/catversion.h | 2 +-
src/include/catalog/index.h | 2 +-
src/include/catalog/pg_index.h | 1 +
src/include/nodes/parsenodes.h | 3 +
src/include/nodes/pathnodes.h | 2 +
src/test/regress/expected/create_index.out | 250 +++++++++++++++++++++
src/test/regress/sql/create_index.sql | 140 ++++++++++++
21 files changed, 672 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index bfb97865e1..61fbf5beb5 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -4590,6 +4590,17 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
partial index.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>indisenabled</structfield> <type>bool</type>
+ </para>
+ <para>
+ If true, the index is currently enabled and should be used for queries.
+ If false, the index is disabled and should not be used for queries,
+ but is still maintained when the table is modified. Default is true.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index e26efec064..613d63b747 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -31,6 +31,8 @@ ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTE
SET STATISTICS <replaceable class="parameter">integer</replaceable>
ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable> [ OWNED BY <replaceable class="parameter">role_name</replaceable> [, ... ] ]
SET TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> [ NOWAIT ]
+ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ENABLE
+ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> DISABLE
</synopsis>
</refsynopsisdiv>
@@ -158,6 +160,29 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>ENABLE</literal></term>
+ <listitem>
+ <para>
+ Enable the specified index. The index will be used by the query planner
+ for query optimization. This is the default state for newly created indexes.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>DISABLE</literal></term>
+ <listitem>
+ <para>
+ Disable the specified index. A disabled index is not used by the query planner
+ for query optimization, but it is still maintained when the underlying table
+ data changes. This can be useful for testing query performance with and without
+ specific indexes, or for temporarily reducing the overhead of index maintenance
+ during bulk data loading operations.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
@@ -300,6 +325,20 @@ CREATE INDEX coord_idx ON measured (x, y, (z + t));
ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000;
</programlisting></para>
+ <para>
+ To enable an index:
+<programlisting>
+ALTER INDEX idx_name ENABLE;
+</programlisting>
+ </para>
+
+ <para>
+ To disable an index:
+<programlisting>
+ALTER INDEX idx_name DISABLE;
+</programlisting>
+ </para>
+
</refsect1>
<refsect1>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 621bc0e253..bc292fe8bb 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -28,6 +28,7 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
[ WITH ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
[ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ]
[ WHERE <replaceable class="parameter">predicate</replaceable> ]
+ [ DISABLE ]
</synopsis>
</refsynopsisdiv>
@@ -586,6 +587,20 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
</para>
</listitem>
</varlistentry>
+
+ <varlistentry>
+ <term><literal>DISABLE</literal></term>
+ <listitem>
+ <para>
+ Creates the index in a disabled state (default enabled). A disabled index is not used by the
+ query planner for query optimization, but it is still maintained when the
+ underlying table data changes. This can be useful when you want to create
+ an index without immediately impacting query performance, allowing you to
+ enable it later at a more convenient time. The index can be enabled later
+ using <command>ALTER INDEX ... ENABLE</command>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
</refsect2>
@@ -701,6 +716,14 @@ Indexes:
partitioned index is a metadata only operation.
</para>
+ <para>
+ When creating an index with the <literal>DISABLE</literal> option, the index
+ will be created but not used for query planning. This can be useful for
+ preparing an index in advance of its use, or for testing purposes. The index
+ will still be maintained as the table is modified, so it can be enabled
+ later without needing to be rebuilt. By default all new indexes are enabled.
+ </para>
+
</refsect2>
</refsect1>
@@ -980,6 +1003,13 @@ SELECT * FROM points
CREATE INDEX CONCURRENTLY sales_quantity_index ON sales_table (quantity);
</programlisting></para>
+ <para>
+ To create an index on the table <literal>test_table</literal> with the default
+ name, but have it initially disabled:
+<programlisting>
+CREATE INDEX ON test_table (col1) DISABLE;
+</programlisting>
+ </para>
</refsect1>
<refsect1>
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index 73a7592fb7..6023d58490 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -302,6 +302,7 @@ Boot_DeclareIndexStmt:
stmt->concurrent = false;
stmt->if_not_exists = false;
stmt->reset_default_tblspc = false;
+ stmt->isenabled = true;
/* locks and races need not concern us in bootstrap mode */
relationId = RangeVarGetRelid(stmt->relation, NoLock,
@@ -355,6 +356,7 @@ Boot_DeclareUniqueIndexStmt:
stmt->concurrent = false;
stmt->if_not_exists = false;
stmt->reset_default_tblspc = false;
+ stmt->isenabled = true;
/* locks and races need not concern us in bootstrap mode */
relationId = RangeVarGetRelid(stmt->relation, NoLock,
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b2b3ecb524..1989cb8ce9 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -118,7 +118,8 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
bool isexclusion,
bool immediate,
bool isvalid,
- bool isready);
+ bool isready,
+ bool isenabled);
static void index_update_stats(Relation rel,
bool hasindex,
double reltuples);
@@ -569,7 +570,8 @@ UpdateIndexRelation(Oid indexoid,
bool isexclusion,
bool immediate,
bool isvalid,
- bool isready)
+ bool isready,
+ bool isenabled)
{
int2vector *indkey;
oidvector *indcollation;
@@ -647,6 +649,7 @@ UpdateIndexRelation(Oid indexoid,
values[Anum_pg_index_indisready - 1] = BoolGetDatum(isready);
values[Anum_pg_index_indislive - 1] = BoolGetDatum(true);
values[Anum_pg_index_indisreplident - 1] = BoolGetDatum(false);
+ values[Anum_pg_index_indisenabled - 1] = BoolGetDatum(isenabled);
values[Anum_pg_index_indkey - 1] = PointerGetDatum(indkey);
values[Anum_pg_index_indcollation - 1] = PointerGetDatum(indcollation);
values[Anum_pg_index_indclass - 1] = PointerGetDatum(indclass);
@@ -712,6 +715,8 @@ UpdateIndexRelation(Oid indexoid,
* already exists.
* INDEX_CREATE_PARTITIONED:
* create a partitioned index (table must be partitioned)
+ * INDEX_CREATE_DISABLED:
+* create the index as disabled if instructed, defaults to being enabled.
* constr_flags: flags passed to index_constraint_create
* (only if INDEX_CREATE_ADD_CONSTRAINT is set)
* allow_system_table_mods: allow table to be a system catalog
@@ -757,6 +762,7 @@ index_create(Relation heapRelation,
bool invalid = (flags & INDEX_CREATE_INVALID) != 0;
bool concurrent = (flags & INDEX_CREATE_CONCURRENT) != 0;
bool partitioned = (flags & INDEX_CREATE_PARTITIONED) != 0;
+ bool isenabled = (flags & INDEX_CREATE_ENABLED) != 0;
char relkind;
TransactionId relfrozenxid;
MultiXactId relminmxid;
@@ -1040,13 +1046,15 @@ index_create(Relation heapRelation,
* (Or, could define a rule to maintain the predicate) --Nels, Feb '92
* ----------------
*/
+
UpdateIndexRelation(indexRelationId, heapRelationId, parentIndexRelid,
indexInfo,
collationIds, opclassIds, coloptions,
isprimary, is_exclusion,
(constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) == 0,
!concurrent && !invalid,
- !concurrent);
+ !concurrent,
+ isenabled);
/*
* Register relcache invalidation on the indexes' heap relation, to
@@ -1315,6 +1323,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
List *indexColNames = NIL;
List *indexExprs = NIL;
List *indexPreds = NIL;
+ Form_pg_index indexForm;
+ bits16 createFlags;
indexRelation = index_open(oldIndexId, RowExclusiveLock);
@@ -1342,6 +1352,9 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
Anum_pg_index_indoption);
indcoloptions = (int2vector *) DatumGetPointer(colOptionDatum);
+ /* Get the enabled state of the original index */
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
/* Fetch reloptions of index if any */
classTuple = SearchSysCache1(RELOID, ObjectIdGetDatum(oldIndexId));
if (!HeapTupleIsValid(classTuple))
@@ -1433,6 +1446,16 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
stattargets[i].isnull = isnull;
}
+ /*
+ * Determine the create flags for the new index.
+ * We always use SKIP_BUILD and CONCURRENT for concurrent reindexing.
+ * If the original index was enabled, we also set the ENABLED flag
+ * to maintain the same state in the new index.
+ */
+ createFlags = INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT;
+ if (indexForm->indisenabled)
+ createFlags |= INDEX_CREATE_ENABLED;
+
/*
* Now create the new index.
*
@@ -1456,7 +1479,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
indcoloptions->values,
stattargets,
reloptionsDatum,
- INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT,
+ createFlags,
0,
true, /* allow table to be a system catalog? */
false, /* is_internal? */
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 738bc46ae8..ce497eed58 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -324,7 +324,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
BTREE_AM_OID,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
- INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_ENABLED, 0, true, true, NULL);
table_close(toast_rel, NoLock);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f99c2d2dee..c8bf2c1fd4 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1191,6 +1191,10 @@ DefineIndex(Oid tableId,
flags |= INDEX_CREATE_PARTITIONED;
if (stmt->primary)
flags |= INDEX_CREATE_IS_PRIMARY;
+ if (stmt->isenabled)
+ flags |= INDEX_CREATE_ENABLED;
+ else
+ flags &= ~INDEX_CREATE_ENABLED;
/*
* If the table is partitioned, and recursion was declined but partitions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2d703aa22e..19b78f2ec2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -664,7 +664,7 @@ static List *GetParentedForeignKeyRefs(Relation partition);
static void ATDetachCheckNoForeignKeyRefs(Relation partition);
static char GetAttributeCompression(Oid atttypid, const char *compression);
static char GetAttributeStorage(Oid atttypid, const char *storagemode);
-
+static void ATExecEnableDisableIndex(Relation rel, bool enable);
/* ----------------------------------------------------------------
* DefineRelation
@@ -4546,6 +4546,8 @@ AlterTableGetLockLevel(List *cmds)
case AT_SetExpression:
case AT_DropExpression:
case AT_SetCompression:
+ case AT_EnableIndex:
+ case AT_DisableIndex:
cmd_lockmode = AccessExclusiveLock;
break;
@@ -5123,6 +5125,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
+ case AT_EnableIndex:
+ case AT_DisableIndex:
+ ATSimplePermissions(cmd->subtype, rel, ATT_INDEX | ATT_PARTITIONED_INDEX);
+ /* No command-specific prep needed */
+ pass = AT_PASS_MISC;
+ break;
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
(int) cmd->subtype);
@@ -5519,6 +5527,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
case AT_DetachPartitionFinalize:
address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name);
break;
+ case AT_EnableIndex:
+ ATExecEnableDisableIndex(rel, true);
+ break;
+ case AT_DisableIndex:
+ ATExecEnableDisableIndex(rel, false);
+ break;
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
(int) cmd->subtype);
@@ -6418,6 +6432,8 @@ alter_table_type_to_string(AlterTableType cmdtype)
return "DROP COLUMN";
case AT_AddIndex:
case AT_ReAddIndex:
+ case AT_EnableIndex:
+ case AT_DisableIndex:
return NULL; /* not real grammar */
case AT_AddConstraint:
case AT_ReAddConstraint:
@@ -20214,3 +20230,79 @@ GetAttributeStorage(Oid atttypid, const char *storagemode)
return cstorage;
}
+
+/*
+ * ATExecEnableDisableIndex function handles the execution of enabling or disabling an index.
+ * It performs an in-place update to preserve the pg_index row's xmin.
+ */
+static void
+ATExecEnableDisableIndex(Relation rel, bool enable)
+{
+ Oid indexOid = RelationGetRelid(rel);
+ Relation pg_index;
+ HeapTuple indexTuple;
+ Form_pg_index indexForm;
+ bool updated = false;
+
+ /* Open pg_index */
+ pg_index = table_open(IndexRelationId, RowExclusiveLock);
+
+ /* Fetch the index's pg_index tuple */
+ indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
+ if (!HeapTupleIsValid(indexTuple))
+ elog(ERROR, "cache lookup failed for index %u", indexOid);
+
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
+ /* Check if the index's current state differs from the desired state */
+ if (indexForm->indisenabled != enable)
+ {
+ HeapTuple copyTuple;
+
+ /* Create a modifiable copy of the tuple */
+ copyTuple = heap_copytuple(indexTuple);
+ indexForm = (Form_pg_index) GETSTRUCT(copyTuple);
+
+ indexForm->indisenabled = enable;
+
+ /* Perform an in-place update */
+ heap_inplace_update(pg_index, copyTuple);
+
+ /* Free the copy */
+ heap_freetuple(copyTuple);
+
+ updated = true;
+
+ /* Update relcache */
+ CacheInvalidateRelcache(rel);
+
+ /*
+ * Invalidate the relcache for the table, so that after we commit
+ * all sessions will refresh the table's index state before
+ * attempting to use this index.
+ */
+ CacheInvalidateRelcache(rel);
+
+ ereport(NOTICE,
+ (errmsg("index \"%s\" is now %s",
+ RelationGetRelationName(rel),
+ enable ? "enabled" : "disabled")));
+ }
+ else
+ {
+ ereport(NOTICE,
+ (errmsg("index \"%s\" is already %s",
+ RelationGetRelationName(rel),
+ enable ? "enabled" : "disabled")));
+ }
+
+ /* Clean up */
+ ReleaseSysCache(indexTuple);
+ table_close(pg_index, RowExclusiveLock);
+
+ /* Invoke the object access hook if we updated the index */
+ if (updated)
+ {
+ InvokeObjectPostAlterHook(IndexRelationId, indexOid, 0);
+ }
+}
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index c0fcc7d78d..843237dadf 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -254,6 +254,10 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
{
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
+ /* Skip disabled indexes */
+ if (!index->enabled)
+ continue;
+
/* Protect limited-size array in IndexClauseSets */
Assert(index->nkeycolumns <= INDEX_MAX_KEYS);
@@ -715,6 +719,10 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
bool skip_nonnative_saop = false;
ListCell *lc;
+ /* Skip disabled indexes */
+ if (!index->enabled)
+ return;
+
/*
* Build simple index paths using the clauses. Allow ScalarArrayOpExpr
* clauses only if the index AM supports them natively.
@@ -823,6 +831,10 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
Assert(skip_nonnative_saop != NULL || scantype == ST_BITMAPSCAN);
+ /* Skip disabled indexes */
+ if (!index->enabled)
+ return NIL;
+
/*
* Check that index supports the desired scan type(s)
*/
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index b913f91ff0..30dc6353f6 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -192,6 +192,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
}
}
+
/*
* Estimate relation size --- unless it's an inheritance parent, in which
* case the size we want is not the rel's own size but the size of its
@@ -459,6 +460,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
info->unique = index->indisunique;
info->immediate = index->indimmediate;
info->hypothetical = false;
+ info->enabled = index->indisenabled;
/*
* Estimate the index size. If it's not a partial index, we lock
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index ab304ca989..8c0aad255d 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -333,7 +333,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <ival> add_drop opt_asc_desc opt_nulls_order
%type <node> alter_table_cmd alter_type_cmd opt_collate_clause
- replica_identity partition_cmd index_partition_cmd
+ replica_identity partition_cmd index_partition_cmd index_alter_cmd
%type <list> alter_table_cmds alter_type_cmds
%type <list> alter_identity_column_option_list
%type <defelt> alter_identity_column_option
@@ -496,6 +496,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <boolean> opt_unique opt_verbose opt_full
%type <boolean> opt_freeze opt_analyze opt_default
%type <defelt> opt_binary copy_delimiter
+%type <boolean> opt_index_enabled
%type <boolean> copy_from opt_program
@@ -2144,6 +2145,24 @@ AlterTableStmt:
n->nowait = $13;
$$ = (Node *) n;
}
+ | ALTER INDEX qualified_name index_alter_cmd
+ {
+ AlterTableStmt *n = makeNode(AlterTableStmt);
+ n->relation = $3;
+ n->cmds = list_make1($4);
+ n->objtype = OBJECT_INDEX;
+ n->missing_ok = false;
+ $$ = (Node *) n;
+ }
+ | ALTER INDEX IF_P EXISTS qualified_name index_alter_cmd
+ {
+ AlterTableStmt *n = makeNode(AlterTableStmt);
+ n->relation = $5;
+ n->cmds = list_make1($6);
+ n->objtype = OBJECT_INDEX;
+ n->missing_ok = true;
+ $$ = (Node *) n;
+ }
| ALTER INDEX qualified_name alter_table_cmds
{
AlterTableStmt *n = makeNode(AlterTableStmt);
@@ -2369,6 +2388,21 @@ index_partition_cmd:
}
;
+index_alter_cmd:
+ /* ALTER INDEX <name> ENABLE|DISABLE */
+ ENABLE_P
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_EnableIndex;
+ $$ = (Node *) n;
+ }
+ | DISABLE_P
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DisableIndex;
+ $$ = (Node *) n;
+ }
+ ;
alter_table_cmd:
/* ALTER TABLE <name> ADD <coldef> */
ADD_P columnDef
@@ -8102,7 +8136,7 @@ defacl_privilege_target:
IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
ON relation_expr access_method_clause '(' index_params ')'
- opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause
+ opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause opt_index_enabled
{
IndexStmt *n = makeNode(IndexStmt);
@@ -8117,6 +8151,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
n->options = $14;
n->tableSpace = $15;
n->whereClause = $16;
+ n->isenabled = $17;
n->excludeOpNames = NIL;
n->idxcomment = NULL;
n->indexOid = InvalidOid;
@@ -8134,7 +8169,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
}
| CREATE opt_unique INDEX opt_concurrently IF_P NOT EXISTS name
ON relation_expr access_method_clause '(' index_params ')'
- opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause
+ opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause opt_index_enabled
{
IndexStmt *n = makeNode(IndexStmt);
@@ -8149,6 +8184,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
n->options = $17;
n->tableSpace = $18;
n->whereClause = $19;
+ n->isenabled = $20;
n->excludeOpNames = NIL;
n->idxcomment = NULL;
n->indexOid = InvalidOid;
@@ -8171,6 +8207,12 @@ opt_unique:
| /*EMPTY*/ { $$ = false; }
;
+opt_index_enabled:
+ ENABLE_P { $$ = true; }
+ | DISABLE_P { $$ = false; }
+ | /*EMPTY*/ { $$ = true; }
+ ;
+
access_method_clause:
USING name { $$ = $2; }
| /*EMPTY*/ { $$ = DEFAULT_INDEX_TYPE; }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 1e15ce10b4..32554612ed 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1588,6 +1588,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
index->concurrent = false;
index->if_not_exists = false;
index->reset_default_tblspc = false;
+ index->isenabled = idxrec->indisenabled;
/*
* We don't try to preserve the name of the source index; instead, just
@@ -2214,6 +2215,8 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
index->concurrent = false;
index->if_not_exists = false;
index->reset_default_tblspc = constraint->reset_default_tblspc;
+ /* Ensure indexes for constraints are created as enabled by default */
+ index->isenabled = true;
/*
* If it's ALTER TABLE ADD CONSTRAINT USING INDEX, look up the index and
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 2177d17e27..05b27ca232 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1559,6 +1559,10 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
else
appendStringInfo(&buf, " WHERE %s", str);
}
+
+ /* Add DISABLE clause if the index is disabled */
+ if (!idxrec->indisenabled)
+ appendStringInfoString(&buf, " DISABLE");
}
/* Clean up */
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 5b6b7b809c..7cd5902ad7 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2344,6 +2344,7 @@ RelationReloadIndexInfo(Relation relation)
relation->rd_index->indisready = index->indisready;
relation->rd_index->indislive = index->indislive;
relation->rd_index->indisreplident = index->indisreplident;
+ relation->rd_index->indisenabled = index->indisenabled;
/* Copy xmin too, as that is needed to make sense of indcheckxmin */
HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data,
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 10bb26f2e4..a20af83da4 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202409211
+#define CATALOG_VERSION_NO 202409220
#endif
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 2dea96f47c..8a47cfd174 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -65,6 +65,7 @@ extern void index_check_primary_key(Relation heapRel,
#define INDEX_CREATE_IF_NOT_EXISTS (1 << 4)
#define INDEX_CREATE_PARTITIONED (1 << 5)
#define INDEX_CREATE_INVALID (1 << 6)
+#define INDEX_CREATE_ENABLED (1 << 7)
extern Oid index_create(Relation heapRelation,
const char *indexRelationName,
@@ -175,7 +176,6 @@ extern void RestoreReindexState(const void *reindexstate);
extern void IndexSetParentIndex(Relation partitionIdx, Oid parentOid);
-
/*
* itemptr_encode - Encode ItemPointer as int64/int8
*
diff --git a/src/include/catalog/pg_index.h b/src/include/catalog/pg_index.h
index 6c89639a9e..b0c7e5f365 100644
--- a/src/include/catalog/pg_index.h
+++ b/src/include/catalog/pg_index.h
@@ -44,6 +44,7 @@ CATALOG(pg_index,2610,IndexRelationId) BKI_SCHEMA_MACRO
bool indisready; /* is this index ready for inserts? */
bool indislive; /* is this index alive at all? */
bool indisreplident; /* is this index the identity for replication? */
+ bool indisenabled; /* is this index enabled for use by queries? */
/* variable-length fields start here, but we allow direct access to indkey */
int2vector indkey BKI_FORCE_NOT_NULL; /* column numbers of indexed cols,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e62ce1b753..e5125895b0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2413,6 +2413,8 @@ typedef enum AlterTableType
AT_SetIdentity, /* SET identity column options */
AT_DropIdentity, /* DROP IDENTITY */
AT_ReAddStatistics, /* internal to commands/tablecmds.c */
+ AT_EnableIndex, /* ENABLE INDEX */
+ AT_DisableIndex, /* DISABLE INDEX */
} AlterTableType;
typedef struct ReplicaIdentityStmt
@@ -3378,6 +3380,7 @@ typedef struct IndexStmt
bool if_not_exists; /* just do nothing if index already exists? */
bool reset_default_tblspc; /* reset default_tablespace prior to
* executing */
+ bool isenabled; /* true if ENABLE (default), false if DISABLE */
} IndexStmt;
/* ----------------------
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 07e2415398..4096c4b797 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1188,6 +1188,8 @@ struct IndexOptInfo
bool immediate;
/* true if index doesn't really exist */
bool hypothetical;
+ /* is index enable? */
+ bool enabled;
/*
* Remaining fields are copied from the index AM's API struct
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index d3358dfc39..57add757aa 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2965,6 +2965,256 @@ ERROR: REINDEX SCHEMA cannot run inside a transaction block
END;
-- concurrently
REINDEX SCHEMA CONCURRENTLY schema_to_reindex;
+-- Test enable/disable functionality for indexes
+-- Setup
+CREATE TABLE enable_disable_test(id int primary key, data text);
+INSERT INTO enable_disable_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+-- CREATE INDEX with ENABLED/DISABLED
+CREATE INDEX enable_disable_idx1 ON enable_disable_test(data) DISABLE;
+CREATE INDEX enable_disable_idx2 ON enable_disable_test(data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- ALTER INDEX ... ENABLE/DISABLE
+-- Before
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+ALTER INDEX enable_disable_idx2 DISABLE;
+NOTICE: index "enable_disable_idx2" is now disabled
+-- After
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | f
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- Enable all indexes
+ALTER INDEX enable_disable_idx2 ENABLE;
+NOTICE: index "enable_disable_idx2" is now enabled
+ALTER INDEX enable_disable_idx1 ENABLE;
+NOTICE: index "enable_disable_idx1" is now enabled
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | t
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- REINDEX TABLE
+REINDEX TABLE enable_disable_test;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | t
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- REINDEX INDEX with enable/disable
+ALTER INDEX enable_disable_idx1 DISABLE;
+NOTICE: index "enable_disable_idx1" is now disabled
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- REINDEX INDEX CONCURRENTLY with enable/disable
+ALTER INDEX enable_disable_idx1 ENABLE;
+NOTICE: index "enable_disable_idx1" is now enabled
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | t
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- Test ENABLE/DISABLE on TOAST index
+CREATE TABLE toast_test (id int primary key, data text);
+INSERT INTO toast_test SELECT g, repeat('long text ', 1000) FROM generate_series(1, 10) g;
+-- Check initial state of TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+ indisvalid | indisready | indislive | indisenabled
+------------+------------+-----------+--------------
+ t | t | t | t
+(1 row)
+
+-- Disable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index DISABLE;
+ERROR: relation "pg_toast.pg_toast_16385_index" does not exist
+-- Check state after disabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+ indisvalid | indisready | indislive | indisenabled
+------------+------------+-----------+--------------
+ t | t | t | t
+(1 row)
+
+-- Enable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index ENABLE;
+ERROR: relation "pg_toast.pg_toast_16385_index" does not exist
+-- Check state after enabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+ indisvalid | indisready | indislive | indisenabled
+------------+------------+-----------+--------------
+ t | t | t | t
+(1 row)
+
+-- Test CREATE TABLE with UNIQUE constraint
+CREATE TABLE unique_constraint_test (id int UNIQUE, data text);
+INSERT INTO unique_constraint_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'unique_constraint_test'::regclass;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+-------------------------------+------------+------------+-----------+--------------
+ unique_constraint_test_id_key | t | t | t | t
+(1 row)
+
+-- Test that the unique constraint index is used
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id = 500;
+ QUERY PLAN
+--------------------------------------------------------------------------
+ Index Scan using unique_constraint_test_id_key on unique_constraint_test
+ Index Cond: (id = 500)
+(2 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id IN (100, 200, 300);
+ QUERY PLAN
+-------------------------------------------------------------
+ Bitmap Heap Scan on unique_constraint_test
+ Recheck Cond: (id = ANY ('{100,200,300}'::integer[]))
+ -> Bitmap Index Scan on unique_constraint_test_id_key
+ Index Cond: (id = ANY ('{100,200,300}'::integer[]))
+(4 rows)
+
+-- Test CREATE TABLE with INDEX
+CREATE TABLE index_test (id int, data text);
+INSERT INTO index_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+CREATE INDEX ON index_test (data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'index_test'::regclass;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+---------------------+------------+------------+-----------+--------------
+ index_test_data_idx | t | t | t | t
+(1 row)
+
+-- Test that the index is used
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+ QUERY PLAN
+------------------------------------------------
+ Bitmap Heap Scan on index_test
+ Recheck Cond: (data = 'data 500'::text)
+ -> Bitmap Index Scan on index_test_data_idx
+ Index Cond: (data = 'data 500'::text)
+(4 rows)
+
+-- Test index usage with joins
+CREATE TABLE join_test (id int PRIMARY KEY, ref_id int);
+INSERT INTO join_test SELECT g, g % 100 FROM generate_series(1, 1000) g;
+EXPLAIN (COSTS OFF)
+SELECT jt.id, it.data
+FROM join_test jt
+JOIN index_test it ON jt.ref_id = it.id
+WHERE jt.id BETWEEN 100 AND 200;
+ QUERY PLAN
+---------------------------------------------------------------
+ Hash Join
+ Hash Cond: (it.id = jt.ref_id)
+ -> Seq Scan on index_test it
+ -> Hash
+ -> Bitmap Heap Scan on join_test jt
+ Recheck Cond: ((id >= 100) AND (id <= 200))
+ -> Bitmap Index Scan on join_test_pkey
+ Index Cond: ((id >= 100) AND (id <= 200))
+(8 rows)
+
+-- Test index usage with ORDER BY
+EXPLAIN (COSTS OFF)
+SELECT *
+FROM index_test
+ORDER BY data
+LIMIT 10;
+ QUERY PLAN
+----------------------------------------------------------
+ Limit
+ -> Index Scan using index_test_data_idx on index_test
+(2 rows)
+
+-- Test disabling an index and its effect on query plan
+ALTER INDEX index_test_data_idx DISABLE;
+NOTICE: index "index_test_data_idx" is now disabled
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+ QUERY PLAN
+-------------------------------------
+ Seq Scan on index_test
+ Filter: (data = 'data 500'::text)
+(2 rows)
+
+-- Re-enable the index
+ALTER INDEX index_test_data_idx ENABLE;
+NOTICE: index "index_test_data_idx" is now enabled
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+ QUERY PLAN
+------------------------------------------------
+ Bitmap Heap Scan on index_test
+ Recheck Cond: (data = 'data 500'::text)
+ -> Bitmap Index Scan on index_test_data_idx
+ Index Cond: (data = 'data 500'::text)
+(4 rows)
+
+-- Clean up
+DROP TABLE enable_disable_test;
+DROP TABLE toast_test;
+DROP TABLE unique_constraint_test;
+DROP TABLE join_test;
+DROP TABLE index_test;
-- Failure for unauthorized user
CREATE ROLE regress_reindexuser NOLOGIN;
SET SESSION ROLE regress_reindexuser;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index fe162cc7c3..d599717c6f 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1297,6 +1297,146 @@ END;
-- concurrently
REINDEX SCHEMA CONCURRENTLY schema_to_reindex;
+-- Test enable/disable functionality for indexes
+
+-- Setup
+CREATE TABLE enable_disable_test(id int primary key, data text);
+INSERT INTO enable_disable_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+
+-- CREATE INDEX with ENABLED/DISABLED
+CREATE INDEX enable_disable_idx1 ON enable_disable_test(data) DISABLE;
+CREATE INDEX enable_disable_idx2 ON enable_disable_test(data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- ALTER INDEX ... ENABLE/DISABLE
+-- Before
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+ALTER INDEX enable_disable_idx2 DISABLE;
+-- After
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- Enable all indexes
+ALTER INDEX enable_disable_idx2 ENABLE;
+ALTER INDEX enable_disable_idx1 ENABLE;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- REINDEX TABLE
+REINDEX TABLE enable_disable_test;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- REINDEX INDEX with enable/disable
+ALTER INDEX enable_disable_idx1 DISABLE;
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- REINDEX INDEX CONCURRENTLY with enable/disable
+ALTER INDEX enable_disable_idx1 ENABLE;
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- Test ENABLE/DISABLE on TOAST index
+CREATE TABLE toast_test (id int primary key, data text);
+INSERT INTO toast_test SELECT g, repeat('long text ', 1000) FROM generate_series(1, 10) g;
+
+-- Check initial state of TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+
+-- Disable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index DISABLE;
+
+-- Check state after disabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+
+-- Enable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index ENABLE;
+
+-- Check state after enabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+
+-- Test CREATE TABLE with UNIQUE constraint
+CREATE TABLE unique_constraint_test (id int UNIQUE, data text);
+INSERT INTO unique_constraint_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'unique_constraint_test'::regclass;
+
+-- Test that the unique constraint index is used
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id = 500;
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id IN (100, 200, 300);
+
+-- Test CREATE TABLE with INDEX
+CREATE TABLE index_test (id int, data text);
+INSERT INTO index_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+CREATE INDEX ON index_test (data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'index_test'::regclass;
+
+-- Test that the index is used
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+
+-- Test index usage with joins
+CREATE TABLE join_test (id int PRIMARY KEY, ref_id int);
+INSERT INTO join_test SELECT g, g % 100 FROM generate_series(1, 1000) g;
+
+EXPLAIN (COSTS OFF)
+SELECT jt.id, it.data
+FROM join_test jt
+JOIN index_test it ON jt.ref_id = it.id
+WHERE jt.id BETWEEN 100 AND 200;
+
+-- Test index usage with ORDER BY
+EXPLAIN (COSTS OFF)
+SELECT *
+FROM index_test
+ORDER BY data
+LIMIT 10;
+
+-- Test disabling an index and its effect on query plan
+ALTER INDEX index_test_data_idx DISABLE;
+
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+
+-- Re-enable the index
+ALTER INDEX index_test_data_idx ENABLE;
+
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+
+-- Clean up
+DROP TABLE enable_disable_test;
+DROP TABLE toast_test;
+DROP TABLE unique_constraint_test;
+DROP TABLE join_test;
+DROP TABLE index_test;
+
-- Failure for unauthorized user
CREATE ROLE regress_reindexuser NOLOGIN;
SET SESSION ROLE regress_reindexuser;
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX
2024-09-10 21:35 Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
2024-09-22 17:42 ` [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
@ 2024-09-22 18:20 ` Shayon Mukherjee <[email protected]>
1 sibling, 0 replies; 6+ messages in thread
From: Shayon Mukherjee @ 2024-09-22 18:20 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected]
Hello,
I realized there were some white spaces in the diff and a compiler warning
error from CI, so I have fixed those and the updated patch with v2 is now
attached.
Shayon
On Sun, Sep 22, 2024 at 1:42 PM Shayon Mukherjee <[email protected]> wrote:
> Hello,
>
> Thank you for all the feedback and insights. Work was busy, so I didn't
> get to follow up earlier.
>
> This patch introduces the ability to enable or disable indexes using ALTER
> INDEX
> and CREATE INDEX commands.
>
> Original motivation for the problem and proposal for a patch
> can be found here[0]
>
> This patch contains the relevant implementation details, new regression
> tests and documentation.
> It passes all the existing specs and the newly added regression tests. It
> compiles, so the
> patch can be applied for testing as well.
>
> I have attached the patch in this email, and have also shared it on my
> Github fork[1]. Mostly so
> that I can ensure the full CI passes.
>
>
> *Implementation details:*
> - New Grammar:
> * ALTER INDEX ... ENABLE/DISABLE
> * CREATE INDEX ... DISABLE
>
> - Default state is enabled. Indexes are only disabled when explicitly
> instructed via CREATE INDEX ... DISABLE or ALTER INDEX ... DISABLE.
>
> - Primary Key and Unique constraint indexes are always enabled as well.
> The
> ENABLE/DISABLE grammar is not supported for these types of indexes. They
> can
> be later disabled via ALTER INDEX ... ENABLE/DISABLE.
>
> - ALTER INDEX ... ENABLE/DISABLE performs an in-place update of the
> pg_index
> catalog to protect against indcheckxmin.
>
> - pg_get_indexdef() support for the new functionality and grammar. This
> change is
> reflected in \d output for tables and pg_dump. We show the DISABLE
> syntax accordingly.
>
> - Updated create_index.sql regression test to cover the new grammar and
> verify
> that disabled indexes are not used in queries.
>
> - Modified get_index_paths() and build_index_paths() to exclude disabled
> indexes from consideration during query planning.
>
> - No changes are made to stop the index from getting rebuilt. This way we
> ensure no
> data miss or corruption when index is re-enabled.
>
> - TOAST indexes are supported and enabled by default.
>
> - REINDEX CONCURRENTLY is supported as well and the existing state of
> pg_index.indisenabled
> is carried over accordingly.
>
> - catversion.h is updated with a new CATALOG_VERSION_NO to reflect change
> in pg_index
> schema.
>
> - See the changes in create_index.sql to get an idea of the grammar and
> sql statements.
>
> - See the changes in create_index.out to get an idea of the catalogue
> states and EXPLAIN
> output to see when an index is getting used or isn't (when disabled).
>
> I am looking forward to any and all feedback on this patch, including but
> not limited to
> code quality, tests, and fundamental logic.
>
> Thank you for the reviews and feedback.
>
> [0]
> https://www.postgresql.org/message-id/CANqtF-oXKe0M%3D0QOih6H%2BsZRjE2BWAbkW_1%2B9nMEAMLxUJg5jA%40ma...
> [1] https://github.com/shayonj/postgres/pull/1
>
> Best,
> Shayon
>
> On Tue, Sep 10, 2024 at 5:35 PM David Rowley <[email protected]> wrote:
>
>> On Wed, 11 Sept 2024 at 03:12, Nathan Bossart <[email protected]>
>> wrote:
>> >
>> > On Tue, Sep 10, 2024 at 10:16:34AM +1200, David Rowley wrote:
>> > > If we get the skip scan feature for PG18, then there's likely going to
>> > > be lots of people with indexes that they might want to consider
>> > > removing after upgrading. Maybe this is a good time to consider this
>> > > feature as it possibly won't ever be more useful than it will be after
>> > > we get skip scans.
>> >
>> > +1, this is something I've wanted for some time. There was some past
>> > discussion, too [0].
>> >
>> > [0]
>> https://postgr.es/m/flat/ed8c9ed7-bb5d-aaec-065b-ad4893645deb%402ndQuadrant.com
>>
>> Thanks for digging that up. I'd forgotten about that. I see there was
>> pushback from having this last time, which is now over 6 years ago.
>> In the meantime, we still have nothing to make this easy for people.
>>
>> I think the most important point I read in that thread is [1]. Maybe
>> what I mentioned in [2] is a good workaround.
>>
>> Additionally, I think there will need to be syntax in CREATE INDEX for
>> this. Without that pg_get_indexdef() might return SQL that does not
>> reflect the current state of the index. MySQL seems to use "CREATE
>> INDEX name ON table (col) [VISIBLE|INVISIBLE]".
>>
>> David
>>
>> [1]
>> https://www.postgresql.org/message-id/20180618215635.m5vrnxdxhxytvmcm%40alap3.anarazel.de
>> [2]
>> https://www.postgresql.org/message-id/CAKJS1f_L7y_BTGESp5Qd6BSRHXP0mj3x9O9C_U27GU478UwpBw%40mail.gma...
>>
>
>
> --
> Kind Regards,
> Shayon Mukherjee
>
--
Kind Regards,
Shayon Mukherjee
Attachments:
[application/octet-stream] v2-0001-Introduce-the-ability-to-enable-disable-indexes.patch (44.9K, ../../CANqtF-rscQ2-7Hks3=VwqNXf6iLh-cRA-iPzC34z9LThajSV8Q@mail.gmail.com/3-v2-0001-Introduce-the-ability-to-enable-disable-indexes.patch)
download | inline diff:
From 848b143d6ae1817602aa68710e9bcc033c061da9 Mon Sep 17 00:00:00 2001
From: Shayon Mukherjee <[email protected]>
Date: Sun, 22 Sep 2024 14:01:45 -0400
Subject: [PATCH v2] Introduce the ability to enable/disable indexes
---
doc/src/sgml/catalogs.sgml | 11 +
doc/src/sgml/ref/alter_index.sgml | 39 ++++
doc/src/sgml/ref/create_index.sgml | 29 +++
src/backend/bootstrap/bootparse.y | 2 +
src/backend/catalog/index.c | 31 ++-
src/backend/catalog/toasting.c | 2 +-
src/backend/commands/indexcmds.c | 4 +
src/backend/commands/tablecmds.c | 94 +++++++-
src/backend/optimizer/path/indxpath.c | 12 +
src/backend/optimizer/util/plancat.c | 2 +
src/backend/parser/gram.y | 48 +++-
src/backend/parser/parse_utilcmd.c | 3 +
src/backend/utils/adt/ruleutils.c | 4 +
src/backend/utils/cache/relcache.c | 1 +
src/include/catalog/catversion.h | 2 +-
src/include/catalog/index.h | 2 +-
src/include/catalog/pg_index.h | 1 +
src/include/nodes/parsenodes.h | 3 +
src/include/nodes/pathnodes.h | 2 +
src/test/regress/expected/create_index.out | 250 +++++++++++++++++++++
src/test/regress/sql/create_index.sql | 140 ++++++++++++
21 files changed, 671 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index bfb97865e1..61fbf5beb5 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -4590,6 +4590,17 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
partial index.
</para></entry>
</row>
+
+ <row>
+ <entry role="catalog_table_entry"><para role="column_definition">
+ <structfield>indisenabled</structfield> <type>bool</type>
+ </para>
+ <para>
+ If true, the index is currently enabled and should be used for queries.
+ If false, the index is disabled and should not be used for queries,
+ but is still maintained when the table is modified. Default is true.
+ </para></entry>
+ </row>
</tbody>
</tgroup>
</table>
diff --git a/doc/src/sgml/ref/alter_index.sgml b/doc/src/sgml/ref/alter_index.sgml
index e26efec064..613d63b747 100644
--- a/doc/src/sgml/ref/alter_index.sgml
+++ b/doc/src/sgml/ref/alter_index.sgml
@@ -31,6 +31,8 @@ ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ALTE
SET STATISTICS <replaceable class="parameter">integer</replaceable>
ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable> [ OWNED BY <replaceable class="parameter">role_name</replaceable> [, ... ] ]
SET TABLESPACE <replaceable class="parameter">new_tablespace</replaceable> [ NOWAIT ]
+ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ENABLE
+ALTER INDEX [ IF EXISTS ] <replaceable class="parameter">name</replaceable> DISABLE
</synopsis>
</refsynopsisdiv>
@@ -158,6 +160,29 @@ ALTER INDEX ALL IN TABLESPACE <replaceable class="parameter">name</replaceable>
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>ENABLE</literal></term>
+ <listitem>
+ <para>
+ Enable the specified index. The index will be used by the query planner
+ for query optimization. This is the default state for newly created indexes.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry>
+ <term><literal>DISABLE</literal></term>
+ <listitem>
+ <para>
+ Disable the specified index. A disabled index is not used by the query planner
+ for query optimization, but it is still maintained when the underlying table
+ data changes. This can be useful for testing query performance with and without
+ specific indexes, or for temporarily reducing the overhead of index maintenance
+ during bulk data loading operations.
+ </para>
+ </listitem>
+ </varlistentry>
+
</variablelist>
</para>
@@ -300,6 +325,20 @@ CREATE INDEX coord_idx ON measured (x, y, (z + t));
ALTER INDEX coord_idx ALTER COLUMN 3 SET STATISTICS 1000;
</programlisting></para>
+ <para>
+ To enable an index:
+<programlisting>
+ALTER INDEX idx_name ENABLE;
+</programlisting>
+ </para>
+
+ <para>
+ To disable an index:
+<programlisting>
+ALTER INDEX idx_name DISABLE;
+</programlisting>
+ </para>
+
</refsect1>
<refsect1>
diff --git a/doc/src/sgml/ref/create_index.sgml b/doc/src/sgml/ref/create_index.sgml
index 621bc0e253..f1eebfa250 100644
--- a/doc/src/sgml/ref/create_index.sgml
+++ b/doc/src/sgml/ref/create_index.sgml
@@ -28,6 +28,7 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
[ WITH ( <replaceable class="parameter">storage_parameter</replaceable> [= <replaceable class="parameter">value</replaceable>] [, ... ] ) ]
[ TABLESPACE <replaceable class="parameter">tablespace_name</replaceable> ]
[ WHERE <replaceable class="parameter">predicate</replaceable> ]
+ [ DISABLE ]
</synopsis>
</refsynopsisdiv>
@@ -380,6 +381,19 @@ CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ [ IF NOT EXISTS ] <replaceable class=
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>DISABLE</literal></term>
+ <listitem>
+ <para>
+ Creates the index in a disabled state (default enabled). A disabled index is not used by the
+ query planner for query optimization, but it is still maintained when the
+ underlying table data changes. This can be useful when you want to create
+ an index without immediately impacting query performance, allowing you to
+ enable it later at a more convenient time. The index can be enabled later
+ using <command>ALTER INDEX ... ENABLE</command>.
+ </para>
+ </listitem>
+ </varlistentry>
</variablelist>
<refsect2 id="sql-createindex-storage-parameters" xreflabel="Index Storage Parameters">
@@ -701,6 +715,14 @@ Indexes:
partitioned index is a metadata only operation.
</para>
+ <para>
+ When creating an index with the <literal>DISABLE</literal> option, the index
+ will be created but not used for query planning. This can be useful for
+ preparing an index in advance of its use, or for testing purposes. The index
+ will still be maintained as the table is modified, so it can be enabled
+ later without needing to be rebuilt. By default all new indexes are enabled.
+ </para>
+
</refsect2>
</refsect1>
@@ -980,6 +1002,13 @@ SELECT * FROM points
CREATE INDEX CONCURRENTLY sales_quantity_index ON sales_table (quantity);
</programlisting></para>
+ <para>
+ To create an index on the table <literal>test_table</literal> with the default
+ name, but have it initially disabled:
+<programlisting>
+CREATE INDEX ON test_table (col1) DISABLE;
+</programlisting>
+ </para>
</refsect1>
<refsect1>
diff --git a/src/backend/bootstrap/bootparse.y b/src/backend/bootstrap/bootparse.y
index 73a7592fb7..6023d58490 100644
--- a/src/backend/bootstrap/bootparse.y
+++ b/src/backend/bootstrap/bootparse.y
@@ -302,6 +302,7 @@ Boot_DeclareIndexStmt:
stmt->concurrent = false;
stmt->if_not_exists = false;
stmt->reset_default_tblspc = false;
+ stmt->isenabled = true;
/* locks and races need not concern us in bootstrap mode */
relationId = RangeVarGetRelid(stmt->relation, NoLock,
@@ -355,6 +356,7 @@ Boot_DeclareUniqueIndexStmt:
stmt->concurrent = false;
stmt->if_not_exists = false;
stmt->reset_default_tblspc = false;
+ stmt->isenabled = true;
/* locks and races need not concern us in bootstrap mode */
relationId = RangeVarGetRelid(stmt->relation, NoLock,
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index b2b3ecb524..1989cb8ce9 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -118,7 +118,8 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
bool isexclusion,
bool immediate,
bool isvalid,
- bool isready);
+ bool isready,
+ bool isenabled);
static void index_update_stats(Relation rel,
bool hasindex,
double reltuples);
@@ -569,7 +570,8 @@ UpdateIndexRelation(Oid indexoid,
bool isexclusion,
bool immediate,
bool isvalid,
- bool isready)
+ bool isready,
+ bool isenabled)
{
int2vector *indkey;
oidvector *indcollation;
@@ -647,6 +649,7 @@ UpdateIndexRelation(Oid indexoid,
values[Anum_pg_index_indisready - 1] = BoolGetDatum(isready);
values[Anum_pg_index_indislive - 1] = BoolGetDatum(true);
values[Anum_pg_index_indisreplident - 1] = BoolGetDatum(false);
+ values[Anum_pg_index_indisenabled - 1] = BoolGetDatum(isenabled);
values[Anum_pg_index_indkey - 1] = PointerGetDatum(indkey);
values[Anum_pg_index_indcollation - 1] = PointerGetDatum(indcollation);
values[Anum_pg_index_indclass - 1] = PointerGetDatum(indclass);
@@ -712,6 +715,8 @@ UpdateIndexRelation(Oid indexoid,
* already exists.
* INDEX_CREATE_PARTITIONED:
* create a partitioned index (table must be partitioned)
+ * INDEX_CREATE_DISABLED:
+* create the index as disabled if instructed, defaults to being enabled.
* constr_flags: flags passed to index_constraint_create
* (only if INDEX_CREATE_ADD_CONSTRAINT is set)
* allow_system_table_mods: allow table to be a system catalog
@@ -757,6 +762,7 @@ index_create(Relation heapRelation,
bool invalid = (flags & INDEX_CREATE_INVALID) != 0;
bool concurrent = (flags & INDEX_CREATE_CONCURRENT) != 0;
bool partitioned = (flags & INDEX_CREATE_PARTITIONED) != 0;
+ bool isenabled = (flags & INDEX_CREATE_ENABLED) != 0;
char relkind;
TransactionId relfrozenxid;
MultiXactId relminmxid;
@@ -1040,13 +1046,15 @@ index_create(Relation heapRelation,
* (Or, could define a rule to maintain the predicate) --Nels, Feb '92
* ----------------
*/
+
UpdateIndexRelation(indexRelationId, heapRelationId, parentIndexRelid,
indexInfo,
collationIds, opclassIds, coloptions,
isprimary, is_exclusion,
(constr_flags & INDEX_CONSTR_CREATE_DEFERRABLE) == 0,
!concurrent && !invalid,
- !concurrent);
+ !concurrent,
+ isenabled);
/*
* Register relcache invalidation on the indexes' heap relation, to
@@ -1315,6 +1323,8 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
List *indexColNames = NIL;
List *indexExprs = NIL;
List *indexPreds = NIL;
+ Form_pg_index indexForm;
+ bits16 createFlags;
indexRelation = index_open(oldIndexId, RowExclusiveLock);
@@ -1342,6 +1352,9 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
Anum_pg_index_indoption);
indcoloptions = (int2vector *) DatumGetPointer(colOptionDatum);
+ /* Get the enabled state of the original index */
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
/* Fetch reloptions of index if any */
classTuple = SearchSysCache1(RELOID, ObjectIdGetDatum(oldIndexId));
if (!HeapTupleIsValid(classTuple))
@@ -1433,6 +1446,16 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
stattargets[i].isnull = isnull;
}
+ /*
+ * Determine the create flags for the new index.
+ * We always use SKIP_BUILD and CONCURRENT for concurrent reindexing.
+ * If the original index was enabled, we also set the ENABLED flag
+ * to maintain the same state in the new index.
+ */
+ createFlags = INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT;
+ if (indexForm->indisenabled)
+ createFlags |= INDEX_CREATE_ENABLED;
+
/*
* Now create the new index.
*
@@ -1456,7 +1479,7 @@ index_concurrently_create_copy(Relation heapRelation, Oid oldIndexId,
indcoloptions->values,
stattargets,
reloptionsDatum,
- INDEX_CREATE_SKIP_BUILD | INDEX_CREATE_CONCURRENT,
+ createFlags,
0,
true, /* allow table to be a system catalog? */
false, /* is_internal? */
diff --git a/src/backend/catalog/toasting.c b/src/backend/catalog/toasting.c
index 738bc46ae8..ce497eed58 100644
--- a/src/backend/catalog/toasting.c
+++ b/src/backend/catalog/toasting.c
@@ -324,7 +324,7 @@ create_toast_table(Relation rel, Oid toastOid, Oid toastIndexOid,
BTREE_AM_OID,
rel->rd_rel->reltablespace,
collationIds, opclassIds, NULL, coloptions, NULL, (Datum) 0,
- INDEX_CREATE_IS_PRIMARY, 0, true, true, NULL);
+ INDEX_CREATE_IS_PRIMARY | INDEX_CREATE_ENABLED, 0, true, true, NULL);
table_close(toast_rel, NoLock);
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index f99c2d2dee..7cd2041300 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -1191,6 +1191,10 @@ DefineIndex(Oid tableId,
flags |= INDEX_CREATE_PARTITIONED;
if (stmt->primary)
flags |= INDEX_CREATE_IS_PRIMARY;
+ if (stmt->isenabled)
+ flags |= INDEX_CREATE_ENABLED;
+ else
+ flags &= ~INDEX_CREATE_ENABLED;
/*
* If the table is partitioned, and recursion was declined but partitions
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 2d703aa22e..905a3f58a2 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -664,7 +664,7 @@ static List *GetParentedForeignKeyRefs(Relation partition);
static void ATDetachCheckNoForeignKeyRefs(Relation partition);
static char GetAttributeCompression(Oid atttypid, const char *compression);
static char GetAttributeStorage(Oid atttypid, const char *storagemode);
-
+static void ATExecEnableDisableIndex(Relation rel, bool enable);
/* ----------------------------------------------------------------
* DefineRelation
@@ -4546,6 +4546,8 @@ AlterTableGetLockLevel(List *cmds)
case AT_SetExpression:
case AT_DropExpression:
case AT_SetCompression:
+ case AT_EnableIndex:
+ case AT_DisableIndex:
cmd_lockmode = AccessExclusiveLock;
break;
@@ -5123,6 +5125,12 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
/* No command-specific prep needed */
pass = AT_PASS_MISC;
break;
+ case AT_EnableIndex:
+ case AT_DisableIndex:
+ ATSimplePermissions(cmd->subtype, rel, ATT_INDEX | ATT_PARTITIONED_INDEX);
+ /* No command-specific prep needed */
+ pass = AT_PASS_MISC;
+ break;
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
(int) cmd->subtype);
@@ -5519,6 +5527,12 @@ ATExecCmd(List **wqueue, AlteredTableInfo *tab,
case AT_DetachPartitionFinalize:
address = ATExecDetachPartitionFinalize(rel, ((PartitionCmd *) cmd->def)->name);
break;
+ case AT_EnableIndex:
+ ATExecEnableDisableIndex(rel, true);
+ break;
+ case AT_DisableIndex:
+ ATExecEnableDisableIndex(rel, false);
+ break;
default: /* oops */
elog(ERROR, "unrecognized alter table type: %d",
(int) cmd->subtype);
@@ -6418,6 +6432,8 @@ alter_table_type_to_string(AlterTableType cmdtype)
return "DROP COLUMN";
case AT_AddIndex:
case AT_ReAddIndex:
+ case AT_EnableIndex:
+ case AT_DisableIndex:
return NULL; /* not real grammar */
case AT_AddConstraint:
case AT_ReAddConstraint:
@@ -20214,3 +20230,79 @@ GetAttributeStorage(Oid atttypid, const char *storagemode)
return cstorage;
}
+
+/*
+ * ATExecEnableDisableIndex function handles the execution of enabling or disabling an index.
+ * It performs an in-place update to preserve the pg_index row's xmin.
+ */
+static void
+ATExecEnableDisableIndex(Relation rel, bool enable)
+{
+ Oid indexOid = RelationGetRelid(rel);
+ Relation pg_index;
+ HeapTuple indexTuple;
+ Form_pg_index indexForm;
+ bool updated = false;
+
+ /* Open pg_index */
+ pg_index = table_open(IndexRelationId, RowExclusiveLock);
+
+ /* Fetch the index's pg_index tuple */
+ indexTuple = SearchSysCache1(INDEXRELID, ObjectIdGetDatum(indexOid));
+ if (!HeapTupleIsValid(indexTuple))
+ elog(ERROR, "cache lookup failed for index %u", indexOid);
+
+ indexForm = (Form_pg_index) GETSTRUCT(indexTuple);
+
+ /* Check if the index's current state differs from the desired state */
+ if (indexForm->indisenabled != enable)
+ {
+ HeapTuple copyTuple;
+
+ /* Create a modifiable copy of the tuple */
+ copyTuple = heap_copytuple(indexTuple);
+ indexForm = (Form_pg_index) GETSTRUCT(copyTuple);
+
+ indexForm->indisenabled = enable;
+
+ /* Perform an in-place update */
+ heap_inplace_update(pg_index, copyTuple);
+
+ /* Free the copy */
+ heap_freetuple(copyTuple);
+
+ updated = true;
+
+ /* Update relcache */
+ CacheInvalidateRelcache(rel);
+
+ /*
+ * Invalidate the relcache for the table, so that after we commit
+ * all sessions will refresh the table's index state before
+ * attempting to use this index.
+ */
+ CacheInvalidateRelcache(rel);
+
+ ereport(NOTICE,
+ (errmsg("index \"%s\" is now %s",
+ RelationGetRelationName(rel),
+ enable ? "enabled" : "disabled")));
+ }
+ else
+ {
+ ereport(NOTICE,
+ (errmsg("index \"%s\" is already %s",
+ RelationGetRelationName(rel),
+ enable ? "enabled" : "disabled")));
+ }
+
+ /* Clean up */
+ ReleaseSysCache(indexTuple);
+ table_close(pg_index, RowExclusiveLock);
+
+ /* Invoke the object access hook if we updated the index */
+ if (updated)
+ {
+ InvokeObjectPostAlterHook(IndexRelationId, indexOid, 0);
+ }
+}
diff --git a/src/backend/optimizer/path/indxpath.c b/src/backend/optimizer/path/indxpath.c
index c0fcc7d78d..843237dadf 100644
--- a/src/backend/optimizer/path/indxpath.c
+++ b/src/backend/optimizer/path/indxpath.c
@@ -254,6 +254,10 @@ create_index_paths(PlannerInfo *root, RelOptInfo *rel)
{
IndexOptInfo *index = (IndexOptInfo *) lfirst(lc);
+ /* Skip disabled indexes */
+ if (!index->enabled)
+ continue;
+
/* Protect limited-size array in IndexClauseSets */
Assert(index->nkeycolumns <= INDEX_MAX_KEYS);
@@ -715,6 +719,10 @@ get_index_paths(PlannerInfo *root, RelOptInfo *rel,
bool skip_nonnative_saop = false;
ListCell *lc;
+ /* Skip disabled indexes */
+ if (!index->enabled)
+ return;
+
/*
* Build simple index paths using the clauses. Allow ScalarArrayOpExpr
* clauses only if the index AM supports them natively.
@@ -823,6 +831,10 @@ build_index_paths(PlannerInfo *root, RelOptInfo *rel,
Assert(skip_nonnative_saop != NULL || scantype == ST_BITMAPSCAN);
+ /* Skip disabled indexes */
+ if (!index->enabled)
+ return NIL;
+
/*
* Check that index supports the desired scan type(s)
*/
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index b913f91ff0..30dc6353f6 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -192,6 +192,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
}
}
+
/*
* Estimate relation size --- unless it's an inheritance parent, in which
* case the size we want is not the rel's own size but the size of its
@@ -459,6 +460,7 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
info->unique = index->indisunique;
info->immediate = index->indimmediate;
info->hypothetical = false;
+ info->enabled = index->indisenabled;
/*
* Estimate the index size. If it's not a partial index, we lock
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index ab304ca989..7e47541541 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -333,7 +333,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <ival> add_drop opt_asc_desc opt_nulls_order
%type <node> alter_table_cmd alter_type_cmd opt_collate_clause
- replica_identity partition_cmd index_partition_cmd
+ replica_identity partition_cmd index_partition_cmd index_alter_cmd
%type <list> alter_table_cmds alter_type_cmds
%type <list> alter_identity_column_option_list
%type <defelt> alter_identity_column_option
@@ -496,6 +496,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type <boolean> opt_unique opt_verbose opt_full
%type <boolean> opt_freeze opt_analyze opt_default
%type <defelt> opt_binary copy_delimiter
+%type <boolean> opt_index_enabled
%type <boolean> copy_from opt_program
@@ -2144,6 +2145,24 @@ AlterTableStmt:
n->nowait = $13;
$$ = (Node *) n;
}
+ | ALTER INDEX qualified_name index_alter_cmd
+ {
+ AlterTableStmt *n = makeNode(AlterTableStmt);
+ n->relation = $3;
+ n->cmds = list_make1($4);
+ n->objtype = OBJECT_INDEX;
+ n->missing_ok = false;
+ $$ = (Node *) n;
+ }
+ | ALTER INDEX IF_P EXISTS qualified_name index_alter_cmd
+ {
+ AlterTableStmt *n = makeNode(AlterTableStmt);
+ n->relation = $5;
+ n->cmds = list_make1($6);
+ n->objtype = OBJECT_INDEX;
+ n->missing_ok = true;
+ $$ = (Node *) n;
+ }
| ALTER INDEX qualified_name alter_table_cmds
{
AlterTableStmt *n = makeNode(AlterTableStmt);
@@ -2369,6 +2388,21 @@ index_partition_cmd:
}
;
+index_alter_cmd:
+ /* ALTER INDEX <name> ENABLE|DISABLE */
+ ENABLE_P
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_EnableIndex;
+ $$ = (Node *) n;
+ }
+ | DISABLE_P
+ {
+ AlterTableCmd *n = makeNode(AlterTableCmd);
+ n->subtype = AT_DisableIndex;
+ $$ = (Node *) n;
+ }
+ ;
alter_table_cmd:
/* ALTER TABLE <name> ADD <coldef> */
ADD_P columnDef
@@ -8102,7 +8136,7 @@ defacl_privilege_target:
IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
ON relation_expr access_method_clause '(' index_params ')'
- opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause
+ opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause opt_index_enabled
{
IndexStmt *n = makeNode(IndexStmt);
@@ -8117,6 +8151,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
n->options = $14;
n->tableSpace = $15;
n->whereClause = $16;
+ n->isenabled = $17;
n->excludeOpNames = NIL;
n->idxcomment = NULL;
n->indexOid = InvalidOid;
@@ -8134,7 +8169,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
}
| CREATE opt_unique INDEX opt_concurrently IF_P NOT EXISTS name
ON relation_expr access_method_clause '(' index_params ')'
- opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause
+ opt_include opt_unique_null_treatment opt_reloptions OptTableSpace where_clause opt_index_enabled
{
IndexStmt *n = makeNode(IndexStmt);
@@ -8149,6 +8184,7 @@ IndexStmt: CREATE opt_unique INDEX opt_concurrently opt_single_name
n->options = $17;
n->tableSpace = $18;
n->whereClause = $19;
+ n->isenabled = $20;
n->excludeOpNames = NIL;
n->idxcomment = NULL;
n->indexOid = InvalidOid;
@@ -8171,6 +8207,12 @@ opt_unique:
| /*EMPTY*/ { $$ = false; }
;
+opt_index_enabled:
+ ENABLE_P { $$ = true; }
+ | DISABLE_P { $$ = false; }
+ | /*EMPTY*/ { $$ = true; }
+ ;
+
access_method_clause:
USING name { $$ = $2; }
| /*EMPTY*/ { $$ = DEFAULT_INDEX_TYPE; }
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 1e15ce10b4..32554612ed 100644
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -1588,6 +1588,7 @@ generateClonedIndexStmt(RangeVar *heapRel, Relation source_idx,
index->concurrent = false;
index->if_not_exists = false;
index->reset_default_tblspc = false;
+ index->isenabled = idxrec->indisenabled;
/*
* We don't try to preserve the name of the source index; instead, just
@@ -2214,6 +2215,8 @@ transformIndexConstraint(Constraint *constraint, CreateStmtContext *cxt)
index->concurrent = false;
index->if_not_exists = false;
index->reset_default_tblspc = constraint->reset_default_tblspc;
+ /* Ensure indexes for constraints are created as enabled by default */
+ index->isenabled = true;
/*
* If it's ALTER TABLE ADD CONSTRAINT USING INDEX, look up the index and
diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 2177d17e27..05b27ca232 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -1559,6 +1559,10 @@ pg_get_indexdef_worker(Oid indexrelid, int colno,
else
appendStringInfo(&buf, " WHERE %s", str);
}
+
+ /* Add DISABLE clause if the index is disabled */
+ if (!idxrec->indisenabled)
+ appendStringInfoString(&buf, " DISABLE");
}
/* Clean up */
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c
index 5b6b7b809c..7cd5902ad7 100644
--- a/src/backend/utils/cache/relcache.c
+++ b/src/backend/utils/cache/relcache.c
@@ -2344,6 +2344,7 @@ RelationReloadIndexInfo(Relation relation)
relation->rd_index->indisready = index->indisready;
relation->rd_index->indislive = index->indislive;
relation->rd_index->indisreplident = index->indisreplident;
+ relation->rd_index->indisenabled = index->indisenabled;
/* Copy xmin too, as that is needed to make sense of indcheckxmin */
HeapTupleHeaderSetXmin(relation->rd_indextuple->t_data,
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index 10bb26f2e4..a20af83da4 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -57,6 +57,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202409211
+#define CATALOG_VERSION_NO 202409220
#endif
diff --git a/src/include/catalog/index.h b/src/include/catalog/index.h
index 2dea96f47c..8a47cfd174 100644
--- a/src/include/catalog/index.h
+++ b/src/include/catalog/index.h
@@ -65,6 +65,7 @@ extern void index_check_primary_key(Relation heapRel,
#define INDEX_CREATE_IF_NOT_EXISTS (1 << 4)
#define INDEX_CREATE_PARTITIONED (1 << 5)
#define INDEX_CREATE_INVALID (1 << 6)
+#define INDEX_CREATE_ENABLED (1 << 7)
extern Oid index_create(Relation heapRelation,
const char *indexRelationName,
@@ -175,7 +176,6 @@ extern void RestoreReindexState(const void *reindexstate);
extern void IndexSetParentIndex(Relation partitionIdx, Oid parentOid);
-
/*
* itemptr_encode - Encode ItemPointer as int64/int8
*
diff --git a/src/include/catalog/pg_index.h b/src/include/catalog/pg_index.h
index 6c89639a9e..b0c7e5f365 100644
--- a/src/include/catalog/pg_index.h
+++ b/src/include/catalog/pg_index.h
@@ -44,6 +44,7 @@ CATALOG(pg_index,2610,IndexRelationId) BKI_SCHEMA_MACRO
bool indisready; /* is this index ready for inserts? */
bool indislive; /* is this index alive at all? */
bool indisreplident; /* is this index the identity for replication? */
+ bool indisenabled; /* is this index enabled for use by queries? */
/* variable-length fields start here, but we allow direct access to indkey */
int2vector indkey BKI_FORCE_NOT_NULL; /* column numbers of indexed cols,
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index e62ce1b753..e5125895b0 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -2413,6 +2413,8 @@ typedef enum AlterTableType
AT_SetIdentity, /* SET identity column options */
AT_DropIdentity, /* DROP IDENTITY */
AT_ReAddStatistics, /* internal to commands/tablecmds.c */
+ AT_EnableIndex, /* ENABLE INDEX */
+ AT_DisableIndex, /* DISABLE INDEX */
} AlterTableType;
typedef struct ReplicaIdentityStmt
@@ -3378,6 +3380,7 @@ typedef struct IndexStmt
bool if_not_exists; /* just do nothing if index already exists? */
bool reset_default_tblspc; /* reset default_tablespace prior to
* executing */
+ bool isenabled; /* true if ENABLE (default), false if DISABLE */
} IndexStmt;
/* ----------------------
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index 07e2415398..4096c4b797 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -1188,6 +1188,8 @@ struct IndexOptInfo
bool immediate;
/* true if index doesn't really exist */
bool hypothetical;
+ /* is index enable? */
+ bool enabled;
/*
* Remaining fields are copied from the index AM's API struct
diff --git a/src/test/regress/expected/create_index.out b/src/test/regress/expected/create_index.out
index d3358dfc39..57add757aa 100644
--- a/src/test/regress/expected/create_index.out
+++ b/src/test/regress/expected/create_index.out
@@ -2965,6 +2965,256 @@ ERROR: REINDEX SCHEMA cannot run inside a transaction block
END;
-- concurrently
REINDEX SCHEMA CONCURRENTLY schema_to_reindex;
+-- Test enable/disable functionality for indexes
+-- Setup
+CREATE TABLE enable_disable_test(id int primary key, data text);
+INSERT INTO enable_disable_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+-- CREATE INDEX with ENABLED/DISABLED
+CREATE INDEX enable_disable_idx1 ON enable_disable_test(data) DISABLE;
+CREATE INDEX enable_disable_idx2 ON enable_disable_test(data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- ALTER INDEX ... ENABLE/DISABLE
+-- Before
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+ALTER INDEX enable_disable_idx2 DISABLE;
+NOTICE: index "enable_disable_idx2" is now disabled
+-- After
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | f
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- Enable all indexes
+ALTER INDEX enable_disable_idx2 ENABLE;
+NOTICE: index "enable_disable_idx2" is now enabled
+ALTER INDEX enable_disable_idx1 ENABLE;
+NOTICE: index "enable_disable_idx1" is now enabled
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | t
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- REINDEX TABLE
+REINDEX TABLE enable_disable_test;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | t
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- REINDEX INDEX with enable/disable
+ALTER INDEX enable_disable_idx1 DISABLE;
+NOTICE: index "enable_disable_idx1" is now disabled
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | f
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- REINDEX INDEX CONCURRENTLY with enable/disable
+ALTER INDEX enable_disable_idx1 ENABLE;
+NOTICE: index "enable_disable_idx1" is now enabled
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+--------------------------+------------+------------+-----------+--------------
+ enable_disable_idx1 | t | t | t | t
+ enable_disable_idx2 | t | t | t | t
+ enable_disable_test_pkey | t | t | t | t
+(3 rows)
+
+-- Test ENABLE/DISABLE on TOAST index
+CREATE TABLE toast_test (id int primary key, data text);
+INSERT INTO toast_test SELECT g, repeat('long text ', 1000) FROM generate_series(1, 10) g;
+-- Check initial state of TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+ indisvalid | indisready | indislive | indisenabled
+------------+------------+-----------+--------------
+ t | t | t | t
+(1 row)
+
+-- Disable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index DISABLE;
+ERROR: relation "pg_toast.pg_toast_16385_index" does not exist
+-- Check state after disabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+ indisvalid | indisready | indislive | indisenabled
+------------+------------+-----------+--------------
+ t | t | t | t
+(1 row)
+
+-- Enable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index ENABLE;
+ERROR: relation "pg_toast.pg_toast_16385_index" does not exist
+-- Check state after enabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+ indisvalid | indisready | indislive | indisenabled
+------------+------------+-----------+--------------
+ t | t | t | t
+(1 row)
+
+-- Test CREATE TABLE with UNIQUE constraint
+CREATE TABLE unique_constraint_test (id int UNIQUE, data text);
+INSERT INTO unique_constraint_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'unique_constraint_test'::regclass;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+-------------------------------+------------+------------+-----------+--------------
+ unique_constraint_test_id_key | t | t | t | t
+(1 row)
+
+-- Test that the unique constraint index is used
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id = 500;
+ QUERY PLAN
+--------------------------------------------------------------------------
+ Index Scan using unique_constraint_test_id_key on unique_constraint_test
+ Index Cond: (id = 500)
+(2 rows)
+
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id IN (100, 200, 300);
+ QUERY PLAN
+-------------------------------------------------------------
+ Bitmap Heap Scan on unique_constraint_test
+ Recheck Cond: (id = ANY ('{100,200,300}'::integer[]))
+ -> Bitmap Index Scan on unique_constraint_test_id_key
+ Index Cond: (id = ANY ('{100,200,300}'::integer[]))
+(4 rows)
+
+-- Test CREATE TABLE with INDEX
+CREATE TABLE index_test (id int, data text);
+INSERT INTO index_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+CREATE INDEX ON index_test (data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'index_test'::regclass;
+ indexrelid | indisvalid | indisready | indislive | indisenabled
+---------------------+------------+------------+-----------+--------------
+ index_test_data_idx | t | t | t | t
+(1 row)
+
+-- Test that the index is used
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+ QUERY PLAN
+------------------------------------------------
+ Bitmap Heap Scan on index_test
+ Recheck Cond: (data = 'data 500'::text)
+ -> Bitmap Index Scan on index_test_data_idx
+ Index Cond: (data = 'data 500'::text)
+(4 rows)
+
+-- Test index usage with joins
+CREATE TABLE join_test (id int PRIMARY KEY, ref_id int);
+INSERT INTO join_test SELECT g, g % 100 FROM generate_series(1, 1000) g;
+EXPLAIN (COSTS OFF)
+SELECT jt.id, it.data
+FROM join_test jt
+JOIN index_test it ON jt.ref_id = it.id
+WHERE jt.id BETWEEN 100 AND 200;
+ QUERY PLAN
+---------------------------------------------------------------
+ Hash Join
+ Hash Cond: (it.id = jt.ref_id)
+ -> Seq Scan on index_test it
+ -> Hash
+ -> Bitmap Heap Scan on join_test jt
+ Recheck Cond: ((id >= 100) AND (id <= 200))
+ -> Bitmap Index Scan on join_test_pkey
+ Index Cond: ((id >= 100) AND (id <= 200))
+(8 rows)
+
+-- Test index usage with ORDER BY
+EXPLAIN (COSTS OFF)
+SELECT *
+FROM index_test
+ORDER BY data
+LIMIT 10;
+ QUERY PLAN
+----------------------------------------------------------
+ Limit
+ -> Index Scan using index_test_data_idx on index_test
+(2 rows)
+
+-- Test disabling an index and its effect on query plan
+ALTER INDEX index_test_data_idx DISABLE;
+NOTICE: index "index_test_data_idx" is now disabled
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+ QUERY PLAN
+-------------------------------------
+ Seq Scan on index_test
+ Filter: (data = 'data 500'::text)
+(2 rows)
+
+-- Re-enable the index
+ALTER INDEX index_test_data_idx ENABLE;
+NOTICE: index "index_test_data_idx" is now enabled
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+ QUERY PLAN
+------------------------------------------------
+ Bitmap Heap Scan on index_test
+ Recheck Cond: (data = 'data 500'::text)
+ -> Bitmap Index Scan on index_test_data_idx
+ Index Cond: (data = 'data 500'::text)
+(4 rows)
+
+-- Clean up
+DROP TABLE enable_disable_test;
+DROP TABLE toast_test;
+DROP TABLE unique_constraint_test;
+DROP TABLE join_test;
+DROP TABLE index_test;
-- Failure for unauthorized user
CREATE ROLE regress_reindexuser NOLOGIN;
SET SESSION ROLE regress_reindexuser;
diff --git a/src/test/regress/sql/create_index.sql b/src/test/regress/sql/create_index.sql
index fe162cc7c3..d599717c6f 100644
--- a/src/test/regress/sql/create_index.sql
+++ b/src/test/regress/sql/create_index.sql
@@ -1297,6 +1297,146 @@ END;
-- concurrently
REINDEX SCHEMA CONCURRENTLY schema_to_reindex;
+-- Test enable/disable functionality for indexes
+
+-- Setup
+CREATE TABLE enable_disable_test(id int primary key, data text);
+INSERT INTO enable_disable_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+
+-- CREATE INDEX with ENABLED/DISABLED
+CREATE INDEX enable_disable_idx1 ON enable_disable_test(data) DISABLE;
+CREATE INDEX enable_disable_idx2 ON enable_disable_test(data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- ALTER INDEX ... ENABLE/DISABLE
+-- Before
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+ALTER INDEX enable_disable_idx2 DISABLE;
+-- After
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- Enable all indexes
+ALTER INDEX enable_disable_idx2 ENABLE;
+ALTER INDEX enable_disable_idx1 ENABLE;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- REINDEX TABLE
+REINDEX TABLE enable_disable_test;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- REINDEX INDEX with enable/disable
+ALTER INDEX enable_disable_idx1 DISABLE;
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- REINDEX INDEX CONCURRENTLY with enable/disable
+ALTER INDEX enable_disable_idx1 ENABLE;
+REINDEX INDEX enable_disable_idx1;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'enable_disable_test'::regclass
+ORDER BY indexrelid::regclass::text;
+
+-- Test ENABLE/DISABLE on TOAST index
+CREATE TABLE toast_test (id int primary key, data text);
+INSERT INTO toast_test SELECT g, repeat('long text ', 1000) FROM generate_series(1, 10) g;
+
+-- Check initial state of TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+
+-- Disable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index DISABLE;
+
+-- Check state after disabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+
+-- Enable TOAST index
+ALTER INDEX pg_toast.pg_toast_16385_index ENABLE;
+
+-- Check state after enabling TOAST index
+SELECT indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indexrelid = (SELECT indexrelid FROM pg_index WHERE indrelid = (SELECT reltoastrelid FROM pg_class WHERE oid = 'toast_test'::regclass));
+
+-- Test CREATE TABLE with UNIQUE constraint
+CREATE TABLE unique_constraint_test (id int UNIQUE, data text);
+INSERT INTO unique_constraint_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'unique_constraint_test'::regclass;
+
+-- Test that the unique constraint index is used
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id = 500;
+EXPLAIN (COSTS OFF) SELECT * FROM unique_constraint_test WHERE id IN (100, 200, 300);
+
+-- Test CREATE TABLE with INDEX
+CREATE TABLE index_test (id int, data text);
+INSERT INTO index_test SELECT g, 'data ' || g FROM generate_series(1, 1000) g;
+CREATE INDEX ON index_test (data);
+SELECT indexrelid::regclass, indisvalid, indisready, indislive, indisenabled
+FROM pg_index
+WHERE indrelid = 'index_test'::regclass;
+
+-- Test that the index is used
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+
+-- Test index usage with joins
+CREATE TABLE join_test (id int PRIMARY KEY, ref_id int);
+INSERT INTO join_test SELECT g, g % 100 FROM generate_series(1, 1000) g;
+
+EXPLAIN (COSTS OFF)
+SELECT jt.id, it.data
+FROM join_test jt
+JOIN index_test it ON jt.ref_id = it.id
+WHERE jt.id BETWEEN 100 AND 200;
+
+-- Test index usage with ORDER BY
+EXPLAIN (COSTS OFF)
+SELECT *
+FROM index_test
+ORDER BY data
+LIMIT 10;
+
+-- Test disabling an index and its effect on query plan
+ALTER INDEX index_test_data_idx DISABLE;
+
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+
+-- Re-enable the index
+ALTER INDEX index_test_data_idx ENABLE;
+
+EXPLAIN (COSTS OFF) SELECT * FROM index_test WHERE data = 'data 500';
+
+-- Clean up
+DROP TABLE enable_disable_test;
+DROP TABLE toast_test;
+DROP TABLE unique_constraint_test;
+DROP TABLE join_test;
+DROP TABLE index_test;
+
-- Failure for unauthorized user
CREATE ROLE regress_reindexuser NOLOGIN;
SET SESSION ROLE regress_reindexuser;
--
2.37.1 (Apple Git-137.1)
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX
2024-09-10 21:35 Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
2024-09-22 17:42 ` [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
@ 2024-09-22 22:44 ` David Rowley <[email protected]>
2024-09-23 11:07 ` Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
1 sibling, 1 reply; 6+ messages in thread
From: David Rowley @ 2024-09-22 22:44 UTC (permalink / raw)
To: Shayon Mukherjee <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected]
On Mon, 23 Sept 2024 at 05:43, Shayon Mukherjee <[email protected]> wrote:
> - Modified get_index_paths() and build_index_paths() to exclude disabled
> indexes from consideration during query planning.
There are quite a large number of other places you also need to modify.
Here are 2 places where the index should be ignored but isn't:
1. expression indexes seem to still be used for statistical estimations:
create table b as select generate_series(1,1000)b;
create index on b((b%10));
analyze b;
explain select distinct b%10 from b;
-- HashAggregate (cost=23.00..23.12 rows=10 width=4)
alter index b_expr_idx disable;
explain select distinct b%10 from b;
-- HashAggregate (cost=23.00..23.12 rows=10 width=4) <-- should be 1000 rows
drop index b_expr_idx;
explain select distinct b%10 from b;
-- HashAggregate (cost=23.00..35.50 rows=1000 width=4)
2. Indexes seem to still be used for join removals.
create table c (c int primary key);
explain select c1.* from c c1 left join c c2 on c1.c=c2.c; --
correctly removes join.
alter index c_pkey disable;
explain select c1.* from c c1 left join c c2 on c1.c=c2.c; -- should
not remove join.
Please carefully look over all places that RelOptInfo.indexlist is
looked at and consider skipping disabled indexes. Please also take
time to find SQL that exercises each of those places so you can verify
that the behaviour is correct after your change. This is also a good
way to learn exactly all cases where indexes are used. Using this
method would have led you to find places like
rel_supports_distinctness(), where you should be skipping disabled
indexes.
The planner should not be making use of disabled indexes for any
optimisations at all.
> - catversion.h is updated with a new CATALOG_VERSION_NO to reflect change in pg_index
> schema.
Please leave that up to the committer. Patch authors doing this just
results in the patch no longer applying as soon as someone commits a
version bump.
Also, please get rid of these notices. The command tag serves that
purpose. It's not interesting that the index is already disabled.
# alter index a_pkey disable;
NOTICE: index "a_pkey" is now disabled
ALTER INDEX
# alter index a_pkey disable;
NOTICE: index "a_pkey" is already disabled
ALTER INDEX
I've only given the code a very quick glance. I don't quite understand
why you're checking the index is enabled in create_index_paths() and
get_index_paths(). I think the check should be done only in
create_index_paths(). Primarily, you'll find code such as "if
(index->indpred != NIL && !index->predOK)" in the locations you need
to consider skipping the disabled index. I think your new code should
be located very close to those places or perhaps within the same if
condition unless it makes it overly complex for the human reader.
I think the documents should also mention that disabling an index is a
useful way to verify an index is not being used before dropping it as
the index can be enabled again at the first sign that performance has
been effected. (It might also be good to mention that checking
pg_stat_user_indexes.idx_scan should be the first port of call when
checking for unused indexes)
David
^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX
2024-09-10 21:35 Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
2024-09-22 17:42 ` [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
2024-09-22 22:44 ` Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
@ 2024-09-23 11:07 ` Shayon Mukherjee <[email protected]>
0 siblings, 0 replies; 6+ messages in thread
From: Shayon Mukherjee @ 2024-09-23 11:07 UTC (permalink / raw)
To: David Rowley <[email protected]>; +Cc: Nathan Bossart <[email protected]>; [email protected]
Hi David,
Thank you so much for the review and pointers. I totally missed expression indexes. I am going to do another proper pass along with your feedback and follow up with an updated patch and any questions.
Excited to be learning so much about the internals.
Shayon
> On Sep 22, 2024, at 6:44 PM, David Rowley <[email protected]> wrote:
>
> On Mon, 23 Sept 2024 at 05:43, Shayon Mukherjee <[email protected]> wrote:
>> - Modified get_index_paths() and build_index_paths() to exclude disabled
>> indexes from consideration during query planning.
>
> There are quite a large number of other places you also need to modify.
>
> Here are 2 places where the index should be ignored but isn't:
>
> 1. expression indexes seem to still be used for statistical estimations:
>
> create table b as select generate_series(1,1000)b;
> create index on b((b%10));
> analyze b;
> explain select distinct b%10 from b;
> -- HashAggregate (cost=23.00..23.12 rows=10 width=4)
>
> alter index b_expr_idx disable;
> explain select distinct b%10 from b;
> -- HashAggregate (cost=23.00..23.12 rows=10 width=4) <-- should be 1000 rows
>
> drop index b_expr_idx;
> explain select distinct b%10 from b;
> -- HashAggregate (cost=23.00..35.50 rows=1000 width=4)
>
> 2. Indexes seem to still be used for join removals.
>
> create table c (c int primary key);
> explain select c1.* from c c1 left join c c2 on c1.c=c2.c; --
> correctly removes join.
> alter index c_pkey disable;
> explain select c1.* from c c1 left join c c2 on c1.c=c2.c; -- should
> not remove join.
>
> Please carefully look over all places that RelOptInfo.indexlist is
> looked at and consider skipping disabled indexes. Please also take
> time to find SQL that exercises each of those places so you can verify
> that the behaviour is correct after your change. This is also a good
> way to learn exactly all cases where indexes are used. Using this
> method would have led you to find places like
> rel_supports_distinctness(), where you should be skipping disabled
> indexes.
>
> The planner should not be making use of disabled indexes for any
> optimisations at all.
>
>> - catversion.h is updated with a new CATALOG_VERSION_NO to reflect change in pg_index
>> schema.
>
> Please leave that up to the committer. Patch authors doing this just
> results in the patch no longer applying as soon as someone commits a
> version bump.
>
> Also, please get rid of these notices. The command tag serves that
> purpose. It's not interesting that the index is already disabled.
>
> # alter index a_pkey disable;
> NOTICE: index "a_pkey" is now disabled
> ALTER INDEX
> # alter index a_pkey disable;
> NOTICE: index "a_pkey" is already disabled
> ALTER INDEX
>
> I've only given the code a very quick glance. I don't quite understand
> why you're checking the index is enabled in create_index_paths() and
> get_index_paths(). I think the check should be done only in
> create_index_paths(). Primarily, you'll find code such as "if
> (index->indpred != NIL && !index->predOK)" in the locations you need
> to consider skipping the disabled index. I think your new code should
> be located very close to those places or perhaps within the same if
> condition unless it makes it overly complex for the human reader.
>
> I think the documents should also mention that disabling an index is a
> useful way to verify an index is not being used before dropping it as
> the index can be enabled again at the first sign that performance has
> been effected. (It might also be good to mention that checking
> pg_stat_user_indexes.idx_scan should be the first port of call when
> checking for unused indexes)
>
> David
^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2024-09-23 11:07 UTC | newest]
Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-08-19 12:34 [PATCH 3/5] pg_rewind: Replace the hybrid list+array data structure with simplehash. Heikki Linnakangas <[email protected]>
2024-09-10 21:35 Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
2024-09-22 17:42 ` [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
2024-09-22 18:20 ` Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[email protected]>
2024-09-22 22:44 ` Re: [PATCH] Re: Proposal to Enable/Disable Index using ALTER INDEX David Rowley <[email protected]>
2024-09-23 11:07 ` Re: [PATCH] Proposal to Enable/Disable Index using ALTER INDEX Shayon Mukherjee <[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