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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: Stephen Frost <sfrost(at)snowman(dot)net>, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Install extensions using update scripts (was Re: Remove superuser() checks from pgstattuple)
Date: 2016-09-09 15:16:56
Message-ID: 22875.1473434216@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Andres Freund <andres(at)anarazel(dot)de> writes:
> On 2016-09-07 09:46:32 -0400, Tom Lane wrote:
>> + * 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.

Hm, I think I had a reason why that was important and not just an
optimization, but right now I don't remember why. Will take a look
and clarify the comment.

>> + /*
>> + * 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.

Yeah, it's an unlikely race condition, but if it did happen we'd hit the
"Assert(!evi_target->installable)" in find_install_path, and then the
"Assert(evi_start != evi_target)" in find_update_path. Maybe that's not
worth worrying about, since it looks like there'd be no ill effects in
non-assert builds: AFAICS we'd correctly deduce that we should use the
now-installable script with no update steps.

>> + 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?

Not sure, what would better wording be?

>> + several versions, for which the author will need to write update scripts.
>> + For example, if you have released a <literal>foo</> extension in
>> + versions <literal>1.0</>, <literal>1.1</>, and <literal>1.2</>, there
>> + should be update scripts <filename>foo--1.0--1.1.sql</>
>> + and <filename>foo--1.1--1.2.sql</>.
>> + Before <productname>PostgreSQL</> 10, it was necessary to create new
>> + script files <filename>foo--1.1.sql</> and <filename>foo--1.2.sql</>
>> + containing the same changes, or else the newer versions could not be

> Maybe replace "same changes" "directly creating the extension's
> contents" or such?

Well, the main point is that you'd have to duplicate the effects of the
update script. Not sure how to improve it without drawing attention
away from that.

Thanks for reviewing!

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2016-09-09 15:23:51 Re: [sqlsmith] Failed assertion in joinrels.c
Previous Message Tom Lane 2016-09-09 14:42:27 Re: Add support for restrictive RLS policies