Re: Doubt about AccessExclusiveLock in ALTER TABLE .. SET ( .. );

> @@ -57,7 +57,8 @@ static relopt_bool boolRelOpts[] =

If we go through this list, I'd rather make informed decisions about
each reloption. Otherwise we're going to get patches for each of them
separately over the next versions.

> @@ -73,7 +75,8 @@ static relopt_bool boolRelOpts[] =
> {
> "fastupdate",
> "Enables \"fast update\" feature for this GIN index",
> - RELOPT_KIND_GIN
> + RELOPT_KIND_GIN,
> + AccessExclusiveLock
> },
> true
> },

> @@ -95,7 +99,8 @@ static relopt_int intRelOpts[] =
> {
> "fillfactor",
> "Packs table pages only to this percentage",
> - RELOPT_KIND_HEAP
> + RELOPT_KIND_HEAP,
> + AccessExclusiveLock
> },
> HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100

> [some other fillfactor settings]

> {
> {
> "gin_pending_list_limit",
> "Maximum size of the pending list for this GIN index, in kilobytes.",
> - RELOPT_KIND_GIN
> + RELOPT_KIND_GIN,
> + AccessExclusiveLock
> },
> -1, 64, MAX_KILOBYTES
> },
> @@ -297,7 +325,8 @@ static relopt_string stringRelOpts[] =
> {
> "buffering",
> "Enables buffering build for this GiST index",
> - RELOPT_KIND_GIST
> + RELOPT_KIND_GIST,
> + AccessExclusiveLock
> },
> 4,
> false,

Why? These options just change things for the future and don't influence
past decisions. It seems unproblematic to change them.

> @@ -259,7 +283,8 @@ static relopt_real realRelOpts[] =
> {
> "seq_page_cost",
> "Sets the planner's estimate of the cost of a sequentially fetched disk page.",
> - RELOPT_KIND_TABLESPACE
> + RELOPT_KIND_TABLESPACE,
> + AccessExclusiveLock
> },
> -1, 0.0, DBL_MAX
> },
> @@ -267,7 +292,8 @@ static relopt_real realRelOpts[] =
> {
> "random_page_cost",
> "Sets the planner's estimate of the cost of a nonsequentially fetched disk page.",
> - RELOPT_KIND_TABLESPACE
> + RELOPT_KIND_TABLESPACE,
> + AccessExclusiveLock
> },
> -1, 0.0, DBL_MAX
> },
> @@ -275,7 +301,8 @@ static relopt_real realRelOpts[] =
> {
> "n_distinct",
> "Sets the planner's estimate of the number of distinct values appearing in a column (excluding child relations).",
> - RELOPT_KIND_ATTRIBUTE
> + RELOPT_KIND_ATTRIBUTE,
> + AccessExclusiveLock
> },
> 0, -1.0, DBL_MAX
> },
> @@ -283,7 +310,8 @@ static relopt_real realRelOpts[] =
> {
> "n_distinct_inherited",
> "Sets the planner's estimate of the number of distinct values appearing in a column (including child relations).",
> - RELOPT_KIND_ATTRIBUTE
> + RELOPT_KIND_ATTRIBUTE,
> + AccessExclusiveLock
> },
> 0, -1.0, DBL_MAX
> },

These probably are the settings that are most interesting to change
without access exlusive locks.

> j = 0;
> for (i = 0; boolRelOpts[i].gen.name; i++)
> + {
> + Assert(DoLockModesConflict(boolRelOpts[i].gen.lockmode,
> + boolRelOpts[i].gen.lockmode));
> j++;
> + }
> for (i = 0; intRelOpts[i].gen.name; i++)
> + {
> + Assert(DoLockModesConflict(intRelOpts[i].gen.lockmode,
> + intRelOpts[i].gen.lockmode));
> j++;
> + }
> for (i = 0; realRelOpts[i].gen.name; i++)
> + {
> + Assert(DoLockModesConflict(realRelOpts[i].gen.lockmode,
> + realRelOpts[i].gen.lockmode));
> j++;
> + }
> for (i = 0; stringRelOpts[i].gen.name; i++)
> + {
> + Assert(DoLockModesConflict(stringRelOpts[i].gen.lockmode,
> + stringRelOpts[i].gen.lockmode));
> j++;
> + }
> j += num_custom_options;

Doesn't really seem worth it to assert individually in each case here to
me.

> +/*
> + * Determine the required LOCKMODE from an option list
> + */
> +LOCKMODE
> +GetRelOptionsLockLevel(List *defList)
> +{
> + LOCKMODE lockmode = NoLock;
> + ListCell *cell;
> +
> + if (defList == NIL)
> + return AccessExclusiveLock;
> +
> + if (need_initialization)
> + initialize_reloptions();
> +
> + foreach(cell, defList)
> + {
> + DefElem *def = (DefElem *) lfirst(cell);
> + int i;
> +
> + for (i = 0; relOpts[i]; i++)
> + {
> + if (pg_strncasecmp(relOpts[i]->name, def->defname, relOpts[i]->namelen + 1) == 0)
> + {
> + if (lockmode < relOpts[i]->lockmode)
> + lockmode = relOpts[i]->lockmode;
> + }
> + }
> + }
> +
> + return lockmode;
> +}

We usually don't compare lock values that way, i.e. there's not
guaranteed to be a strict monotonicity between lock levels. I don't
really agree with that policy, but it's nonetheless there.

Greetings,

Andres Freund