Date: Thu, 1 Feb 2018 14:03:48 +0900
From: Michael Paquier <michael.paquier@gmail.com>
To: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>
Cc: Jaime Casanova <jaime.casanova@2ndquadrant.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: [HACKERS] generated columns
Message-ID: <20180201050348.GA6398@paquier.xyz>
References: <b151f851-4019-bdb1-699e-ebab07d2f40a@2ndquadrant.com>
 <CAJGNTeOxeQPFpEr35PME4yLayRzP_wwzt70kFrPztSTWHf-HDA@mail.gmail.com>
 <CAJGNTeM+MCjA4dBhAdjJ-vR_CwY3u65cCcc+KvnahzLHZjxjbw@mail.gmail.com>
 <1bc948f8-3bab-6c54-50af-c0723a75b85f@2ndquadrant.com>
 <20180116063546.GA3679@paquier.xyz>
 <48502d9b-60c4-5b10-40a2-e34f562132b4@2ndquadrant.com>
 <20180119051804.GC12826@paquier.xyz>
 <2e3d5147-16f8-af0f-00ab-4c72cafc896f@2ndquadrant.com>
 <20180131131804.GB1586@paquier.xyz>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="aM3YZ0Iwxop3KEKx"
Content-Disposition: inline
In-Reply-To: <20180131131804.GB1586@paquier.xyz>
Precedence: bulk


--aM3YZ0Iwxop3KEKx
Content-Type: multipart/mixed; boundary="FL5UXtIhxfXey3p5"
Content-Disposition: inline


--FL5UXtIhxfXey3p5
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Wed, Jan 31, 2018 at 10:18:04PM +0900, Michael Paquier wrote:
> On Thu, Jan 25, 2018 at 10:26:55PM -0500, Peter Eisentraut wrote:
>> On 1/19/18 00:18, Michael Paquier wrote:
>>> +SELECT a, c FROM gtest12;  -- FIXME: should be allowed
>>> +ERROR:  permission denied for function gf1
>>=20
>> This is quite hard to fix and I would like to leave this for a future
>> release.
>=20
> I have been looking at that case more closely again, and on the contrary
> I would advocate that your patch is doing the *right* thing.  In short,
> if the generation expression uses a function and the user has only been
> granted access to read the values, it seems to me that it we should
> require that this user also has the right to execute the function. Would
> that be too user-unfriendly?  I think that this could avoid mistakes
> about giving access to unwanted functions when willing to just give a
> SELECT right as the function could be doing more operations.

Attached is the SQL file I used with test cases for the review.  I
forgot to attach it yesterday.

> Hm.  Identity columns and default columns are part of rowtypes. STORED
> columns can alsobe part of a row type with little effort, so in order to
> be consistent with the all the existing behaviors, it seems to me that
> virtually-generated columns should be part of it as well.  I have
> compiled in the attached SQL file how things behave with your
> patch.  Only virtually-generated columns show a blank value.

The tests used are attached.
--
Michael

--FL5UXtIhxfXey3p5
Content-Type: application/x-sql
Content-Disposition: attachment; filename="generated_cases.sql"
Content-Transfer-Encoding: quoted-printable

-- Triggers with new and old rows=0ACREATE FUNCTION trigger_show_row() RETU=
RNS trigger=0ALANGUAGE plpgsql=0AAS $$=0ABEGIN=0A  IF tg_op IN ('DELETE', '=
UPDATE') THEN=0A    RAISE INFO '%: old =3D %', TG_NAME, OLD;=0A  END IF;=0A=
  IF tg_op IN ('INSERT', 'UPDATE') THEN=0A    RAISE INFO '%: new =3D %', TG=
_NAME, NEW;=0A  END IF;=0A  IF tg_op =3D 'DELETE' THEN=0A    RETURN OLD;=0A=
  ELSE=0A    RETURN NEW;=0A  END IF;=0AEND $$;=0Acreate table tab_generated=
 (a int,=0A  b int generated always as identity,=0A  c int default 0,=0A  d=
 int GENERATED ALWAYS AS (c * 2));=0Acreate trigger tab_before=0A  after in=
sert or update or delete=0A  on tab_generated for each row=0A  execute proc=
edure trigger_show_row();=0Acreate trigger tab_after=0A  before insert or u=
pdate or delete=0A  on tab_generated for each row=0A  execute procedure tri=
gger_show_row();=0Ainsert into tab_generated values (1);=0Aupdate tab_gener=
ated set a =3D 2 where a =3D 1;=0Adelete from tab_generated where a =3D 2;=
=0A=0A-- ACLs with virtual column using functions=0ACREATE FUNCTION dummy_i=
nt(integer) RETURNS integer=0AAS 'select $1;'=0ALANGUAGE SQL=0AIMMUTABLE ST=
RICT;=0ACREATE TABLE tab_generated_acl (=0A  a int PRIMARY KEY,=0A  b int G=
ENERATED ALWAYS AS (dummy_int(a)));=0AREVOKE ALL ON FUNCTION dummy_int(int)=
 FROM PUBLIC;=0AINSERT INTO tab_generated_acl VALUES (1);=0ACREATE ROLE pop=
o;=0AGRANT SELECT (a, b) ON tab_generated_acl TO popo;=0ASET ROLE popo;=0A-=
- should not work as no permission to use function=0ASELECT * FROM tab_gene=
rated_acl;=0ARESET role;=0A-- With default expression=0ACREATE TABLE tab_de=
fault_acl (=0A  a int PRIMARY KEY,=0A  b int DEFAULT (dummy_int(1)));=0AGRA=
NT INSERT (a, b) ON tab_default_acl TO popo;                               =
                                                                           =
                =0ASET ROLE popo;=0A-- should not work as no permission to =
use function=0AINSERT INTO tab_default_acl VALUES (1);=0A
--FL5UXtIhxfXey3p5--

--aM3YZ0Iwxop3KEKx
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAlpyn7QACgkQnvQgOdby
QH1n0w/+JLTcH7hgoZZItJNnwAr2hlVonG8tOSVWpPGX1FotMPAFlaGhUMUmLfcy
GyPZG1/K1cC3RtWsJsyuT+h8/Ml382uUedBMEKntcf4OUgVZYZhoCTCIFCB3Jcb4
zzHdzOJ/G09u7Ybne3rOOxIg71pCJMAAvPfdG8DAg5rRd78J0lA0z4hutXJE6Pss
M1SbWzaprJOXamQ0uqehSNyY6ekqIx7aU0mlB9BNITaUgDu8YeARRsPC0uNObKaf
ncgvNwTOMnyVMe427hiuU80yPhHLoP3hQHl7j4urAQVodeUJLnBkKF7EMvulzFgN
A4XPyVicMTd03S/+msX19XLrqQgeceiQj9KQU/WUinFZ03SlgdUXkRVHhCQ3DloB
TAVsAKANCc1n83WQPcGuO1HUncmrumK5X4UK7su6j0u/2UDLf/5UJVbi3Y5RYOTg
h8nUQkvIM7BjUk+Ipuud/MnVW3HY6wC9bG7GcqyKIjoNg6KUM9c5yWaQDn1ms3pT
+je7DOdDZqIBz2EpAuZPc24/Z8m9MS0BqSTtY6dYoS+WiNo6gn0uwSWbKqGqclDr
n3FQ92UC2boeb/BqUsDf1mjb+syn+KQrEpjNJPKdXDuRrqT2x/ixtlnIvxfUsS3D
R0ZdgtGu/kA0EGitUJv4C7xhBdXziJStUGKeYoGGSxKshNsa8yM=
=YEIN
-----END PGP SIGNATURE-----

--aM3YZ0Iwxop3KEKx--