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


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to