MIME-Version: 1.0
References: 
 <CAGECzQQzDqDzakBkR71ZkQ1N1ffTjAaruRSqppQAKu3WF+6rNQ@mail.gmail.com>
 <dbd3cc5a5edde04941c8624dde7e4bd2a063f366.camel@j-davis.com>
 <CAGECzQTe3y6xwP9aMbFBCxKnQEEn3G3=cEDqoD5xaOwPwypd_A@mail.gmail.com>
 <CAGECzQTAgtTp3G=Em3xEY=T=uiKfyu5xLYri9By=sfNBS5C_9A@mail.gmail.com>
 <CAKFQuwaT4_n=e0YKBZAyox1CQUra2ka0cySs+3pGZR5p50pn-g@mail.gmail.com>
 <CAGECzQTOJrnnJkmMe9nems0jouiKUbFcEb1rb9kE_svsAZiGQg@mail.gmail.com>
 <585e996c-a5c6-4e61-acc4-d92b7a1458ea@vondra.me>
 <CAGECzQS02M6YPDXemo36tShO-ZYObjqnyTJyVttua1PGyN4xRw@mail.gmail.com>
In-Reply-To: 
 <CAGECzQS02M6YPDXemo36tShO-ZYObjqnyTJyVttua1PGyN4xRw@mail.gmail.com>
From: Artem Gavrilov <artem.gavrilov@percona.com>
Date: Wed, 23 Jul 2025 19:12:42 +0200
Message-ID: 
 <CAFPkQKzALOTTBrhj2qDHwVxZQyjF5Xg_P9M=Tn_Dcm3vr=xdTA@mail.gmail.com>
Subject: Re: Extension security improvement: Add support for extensions with
 an owned schema
To: Jelte Fennema-Nio <me@jeltef.nl>
Cc: Tomas Vondra <tomas@vondra.me>,
 "David G. Johnston" <david.g.johnston@gmail.com>,
	Jeff Davis <pgsql@j-davis.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>
Content-Type: multipart/alternative; boundary="000000000000ca4687063a9bd178"
Archived-At: 
 <https://www.postgresql.org/message-id/CAFPkQKzALOTTBrhj2qDHwVxZQyjF5Xg_P9M%3DTn_Dcm3vr%3DxdTA%40mail.gmail.com>
Precedence: bulk

--000000000000ca4687063a9bd178
Content-Type: text/plain; charset="UTF-8"

Hello Jelte,

I reviewed your patch. Overall it looks good, I didn't find any problems
with code. Documentation is in place and clear.

Initial Run
===========
The patch applies cleanly to HEAD (196063d6761). All tests successfully
pass.

Comments
===========
1) I noticed that pg_dump changes weren't covered with tests.

2) I assume these error messages may be confusing, especially first one:

> -- Fails for an already existing schema to be provided
> CREATE EXTENSION test_ext_owned_schema SCHEMA test_ext_owned_schema;
> ERROR:  schema "test_ext_owned_schema" already exists
> -- Fails because a different schema is set in control file
> CREATE EXTENSION test_ext_owned_schema SCHEMA test_schema;
> ERROR:  extension "test_ext_owned_schema" must be installed in schema
> "test_ext_owned_schema"


In both cases it's not clear that the extension requires schema ownership.
Can hint messages be added there?

-- 

Artem Gavrilov

Senior Software Engineer, Percona

artem.gavrilov@percona.com
percona.com <http://www.percona.com>

--000000000000ca4687063a9bd178
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><div dir=3D"ltr"><div>He=
llo Jelte,</div><div><br></div><div>I reviewed your patch. Overall it looks=
 good, I didn&#39;t find any problems with code. Documentation is in place =
and clear.</div><div><br></div><div>Initial Run<br>=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D</div><div>The patch applies cleanly to HEAD (196063d6761). All te=
sts=C2=A0successfully pass.</div><div><br></div><div>Comments</div><div>=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D</div><div>1)=C2=A0I noticed that pg_dump cha=
nges weren&#39;t covered with tests.</div><div><br></div><div>2) I assume t=
hese error messages may be confusing, especially first one:=C2=A0</div><blo=
ckquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left=
:1px solid rgb(204,204,204);padding-left:1ex">-- Fails for an already exist=
ing schema to be provided<br>CREATE EXTENSION test_ext_owned_schema SCHEMA =
test_ext_owned_schema;<br>ERROR: =C2=A0schema &quot;test_ext_owned_schema&q=
uot; already exists<br>-- Fails because a different schema is set in contro=
l file<br>CREATE EXTENSION test_ext_owned_schema SCHEMA test_schema;<br>ERR=
OR: =C2=A0extension &quot;test_ext_owned_schema&quot; must be installed in =
schema &quot;test_ext_owned_schema&quot;</blockquote><div><br></div><div><d=
iv>In both cases it&#39;s not clear that the extension requires schema owne=
rship. Can hint messages be added there?</div><div><br></div></div></div><s=
pan class=3D"gmail_signature_prefix">-- </span><br><div dir=3D"ltr" class=
=3D"gmail_signature" data-smartmail=3D"gmail_signature"><div dir=3D"ltr"><p=
 style=3D"line-height:1.38;margin-top:0pt;margin-bottom:0pt">Artem Gavrilov=
</p><p dir=3D"ltr" style=3D"line-height:1.38;margin-top:0pt;margin-bottom:0=
pt"><span style=3D"font-size:10pt;font-family:Poppins,sans-serif;color:#000=
000;background-color:transparent;font-weight:400;font-style:normal;font-var=
iant:normal;text-decoration:none;vertical-align:baseline;white-space:pre;wh=
ite-space:pre-wrap">Senior Software Engineer, Percona</span></p><br><p dir=
=3D"ltr" style=3D"line-height:1.38;margin-top:0pt;margin-bottom:0pt"><span =
style=3D"font-size:10pt;font-family:Poppins,sans-serif;color:#000000;backgr=
ound-color:transparent;font-weight:400;font-style:normal;font-variant:norma=
l;text-decoration:none;vertical-align:baseline;white-space:pre;white-space:=
pre-wrap"><a href=3D"mailto:artem.gavrilov@percona.com" target=3D"_blank">a=
rtem.gavrilov@percona.com</a></span></p><a href=3D"http://www.percona.com" =
style=3D"text-decoration:none" target=3D"_blank"><span style=3D"font-size:9=
pt;font-family:Poppins,sans-serif;color:#1155cc;background-color:transparen=
t;font-weight:400;font-style:normal;font-variant:normal;text-decoration:und=
erline;vertical-align:baseline;white-space:pre;white-space:pre-wrap">percon=
a.com</span><span style=3D"font-size:10pt;font-family:Poppins,sans-serif;co=
lor:#000000;background-color:transparent;font-weight:400;font-style:normal;=
font-variant:normal;text-decoration:none;vertical-align:baseline;white-spac=
e:pre;white-space:pre-wrap">
</span></a></div></div></div>
</div>
</div>

--000000000000ca4687063a9bd178--