Re: Install extensions using update scripts (was Re: Remove superuser() checks from pgstattuple)

Hi,

On 2016-09-07 09:46:32 -0400, Tom Lane wrote:
> At this point it's awfully tempting to make ALTER EXTENSION UPDATE grow
> a CASCADE option to allow automatic installation of new requirements
> of the new version, but I didn't do that here.

Sounds like a plan. After the refactoring that should be easy.

> @@ -1086,6 +1097,9 @@ identify_update_path(ExtensionControlFil
> * Apply Dijkstra's algorithm to find the shortest path from evi_start to
> * evi_target.
> *
> + * If reject_indirect is true, ignore paths that go through installable
> + * versions (presumably, caller will consider starting from such versions).

Reading this I was initially confused, only after reading
find_install_path() this made sense. It's to cut the search short,
right? Rephrasing this a bit might make sense.

> + /*
> + * No FROM, so we're installing from scratch. If there is an install
> + * script for the desired version, we only need to run that one.
> + */
> + char *filename;
> + struct stat fst;
> +
> oldVersionName = NULL;
> - updateVersions = NIL;
> +
> + filename = get_extension_script_filename(pcontrol, NULL, versionName);
> + if (stat(filename, &fst) == 0)
> + {
> + /* Easy, no extra scripts */
> + updateVersions = NIL;
> + }
> + else
> + {
> + /* Look for best way to install this version */
> + List *evi_list;
> + ExtensionVersionInfo *evi_target;
> +
> + /* Extract the version update graph from the script directory */
> + evi_list = get_ext_ver_list(pcontrol);
> +
> + /* Identify the target version */
> + evi_target = get_ext_ver_info(versionName, &evi_list);
> +
> + /*
> + * We don't expect it to be installable, but maybe somebody added
> + * a suitable script right after our stat() test.
> + */
> + if (evi_target->installable)
> + {
> + /* Easy, no extra scripts */
> + updateVersions = NIL;
> + }

Heh, that's an odd thing to handle.

> + else
> + {
> + /* Identify best path to reach target */
> + ExtensionVersionInfo *evi_start;
> +
> + evi_start = find_install_path(evi_list, evi_target,
> + &updateVersions);
> +
> + /* Fail if no path ... */
> + if (evi_start == NULL)
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("extension \"%s\" has no installation script for version \"%s\"",
> + pcontrol->name, versionName)));

Maybe, and I mean maybe, we should rephrase this to hint at indirect
installability?

Looks good to me.

Greetings,

Andres Freund