Re: Install extensions using update scripts (was Re: Remove superuser() checks from pgstattuple) - Mailing list pgsql-hackers

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