Binary in/out for aclitem

Lists: pgsql-hackers
From: Radosław Smogura <rsmogura(at)softperience(dot)eu>
To: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Binary in/out for aclitem
Date: 2011-02-22 21:30:41
Message-ID: 201102222230.41081.rsmogura@softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi,

Actaully one more POD left it's aclitem :). In Java for e.g. it is used to
obtain column priviliges, I assume some folks may want to use it too.

I tested only recv :-(

Acually I don't know if idea of such format is OK, but my intention was to
send roles names, so driver don't need to ask for name (like for regproc) and
to keep together human-based approch (text) and binary approch.

Kind regards,
Radek

Attachment Content-Type Size
aclitem_binary_20110222.patch text/x-patch 10.1 KB

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Radosław Smogura <rsmogura(at)softperience(dot)eu>
Cc: PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-22 22:04:19
Message-ID: 22627.1298412259@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

=?utf-8?q?Rados=C5=82aw_Smogura?= <rsmogura(at)softperience(dot)eu> writes:
> Actaully one more POD left it's aclitem :). In Java for e.g. it is used to
> obtain column priviliges, I assume some folks may want to use it too.

I think this one has got far less use-case than the other, and I don't
want to expose the internal representation of ACLITEM to the world.
So, nope, not excited about it.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Radosław Smogura <rsmogura(at)softperience(dot)eu>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-22 22:13:17
Message-ID: 4D6434FD.3000605@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 02/22/2011 05:04 PM, Tom Lane wrote:
> =?utf-8?q?Rados=C5=82aw_Smogura?=<rsmogura(at)softperience(dot)eu> writes:
>> Actaully one more POD left it's aclitem :). In Java for e.g. it is used to
>> obtain column priviliges, I assume some folks may want to use it too.
> I think this one has got far less use-case than the other, and I don't
> want to expose the internal representation of ACLITEM to the world.
> So, nope, not excited about it.
>

The sendv for enums sends the label, and ISTR there are some others that
send the text representation also. Would that be better?

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Radosław Smogura <rsmogura(at)softperience(dot)eu>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-22 22:24:28
Message-ID: 23049.1298413468@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 02/22/2011 05:04 PM, Tom Lane wrote:
>> I think this one has got far less use-case than the other, and I don't
>> want to expose the internal representation of ACLITEM to the world.

> The sendv for enums sends the label, and ISTR there are some others that
> send the text representation also. Would that be better?

It'd be more future-proof than this patch, but I'm still unconvinced
about the use-case.

regards, tom lane


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Radosław Smogura <rsmogura(at)softperience(dot)eu>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 01:12:38
Message-ID: AANLkTimH_-AxRGfEfF69Kk0ZMcCDG_jJ9xgiDajajdS-@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> On 02/22/2011 05:04 PM, Tom Lane wrote:
>>> I think this one has got far less use-case than the other, and I don't
>>> want to expose the internal representation of ACLITEM to the world.
>
>> The sendv for enums sends the label, and ISTR there are some others that
>> send the text representation also. Would that be better?
>
> It'd be more future-proof than this patch, but I'm still unconvinced
> about the use-case.

Do we want to intentionally make binary format a second-class citizen?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, Radosław Smogura <rsmogura(at)softperience(dot)eu>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 01:20:39
Message-ID: 9538.1298424039@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> It'd be more future-proof than this patch, but I'm still unconvinced
>> about the use-case.

> Do we want to intentionally make binary format a second-class citizen?

Well, it's not exactly a first-class citizen; compare for instance the
amount of verbiage in the docs about text I/O formats versus the amount
about binary formats. But my question isn't about that; it's about why
aclitem should be considered a first-class citizen. It makes me
uncomfortable that client apps are looking at it at all, because any
that do are bound to get broken in the future, even assuming that they
get the right answers today. I wonder how many such clients are up to
speed for per-column privileges and non-constant default privileges for
instance. And sepgsql is going to cut them off at the knees.

regards, tom lane


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 01:47:55
Message-ID: AANLkTimNOkPiA2cRLQAZi=QVjE2kynqqf-=gv+FLAGFc@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

[ removing Radoslaw from the CC list, as his email is bouncing every time ]

On Tue, Feb 22, 2011 at 8:20 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> It'd be more future-proof than this patch, but I'm still unconvinced
>>> about the use-case.
>
>> Do we want to intentionally make binary format a second-class citizen?
>
> Well, it's not exactly a first-class citizen; compare for instance the
> amount of verbiage in the docs about text I/O formats versus the amount
> about binary formats.  But my question isn't about that; it's about why
> aclitem should be considered a first-class citizen.  It makes me
> uncomfortable that client apps are looking at it at all, because any
> that do are bound to get broken in the future, even assuming that they
> get the right answers today.  I wonder how many such clients are up to
> speed for per-column privileges and non-constant default privileges for
> instance.  And sepgsql is going to cut them off at the knees.

Well, unfortunately, there's an awful lot of information that can only
be obtained in a reasonable way by introspection of the system
catalogs. If you want to know whether user A can select from table B,
there's really no sensible way of obtaining that without parsing the
aclitem entries in some fashion, and unfortunately that's just the tip
of the iceberg. One of the first applications I wrote for PG included
a tool to upgrade the production schema to match the dev schema, which
promptly got broken when (I'm dating myself here[1]) PG added support
for dropping columns (7.3) and recording the grantor on aclitems
(7.4). I'm not going to claim that there aren't better ways of trying
to solve the problems that I was trying to solve that day, but at the
time it seemed like the best solution, and if I had a dollar for every
other person who is written a similar application, I am pretty sure I
could afford at least a pizza, if not dinner at Fogo de Chao.

Now, if you were to propose adding a well-designed set of DCL commands
to expose this kind of information to clients in a more structured
way, I would be the first to applaud. LIST TABLES? SHOW GRANTS TO?
Sign me up! (I got a request for the latter just today.) But until
then, if you need this information, you don't have much choice but to
pull it out of the system catalogs; and if JDBC would rather use
binary format to talk to the server, I don't particularly see any
reason to say "no". If we prefer to expose the text format rather
than anything else, that's OK with me, but I do think it would make
sense to expose something.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

[1] Insert obligatory joke about how no one else would.


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 02:26:27
Message-ID: 10854.1298427987@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Tue, Feb 22, 2011 at 8:20 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> ... But my question isn't about that; it's about why
>> aclitem should be considered a first-class citizen. It makes me
>> uncomfortable that client apps are looking at it at all, because any
>> that do are bound to get broken in the future, even assuming that they
>> get the right answers today. I wonder how many such clients are up to
>> speed for per-column privileges and non-constant default privileges for
>> instance. And sepgsql is going to cut them off at the knees.

> Well, unfortunately, there's an awful lot of information that can only
> be obtained in a reasonable way by introspection of the system
> catalogs. If you want to know whether user A can select from table B,
> there's really no sensible way of obtaining that without parsing the
> aclitem entries in some fashion, and unfortunately that's just the tip
> of the iceberg.

Um, that question is precisely why we invented the has_foo_privilege
class of functions. I would *much* rather see users applying those
functions than looking at ACLs directly --- and if there's some
reasonable use-case that those don't cover, let's talk about that.

> Now, if you were to propose adding a well-designed set of DCL commands
> to expose this kind of information to clients in a more structured
> way, I would be the first to applaud. LIST TABLES? SHOW GRANTS TO?
> Sign me up! (I got a request for the latter just today.) But until
> then, if you need this information, you don't have much choice but to
> pull it out of the system catalogs; and if JDBC would rather use
> binary format to talk to the server, I don't particularly see any
> reason to say "no". If we prefer to expose the text format rather
> than anything else, that's OK with me, but I do think it would make
> sense to expose something.

Well, to go back to the binary-format issue, if we're going to insist
that all standard types have binary I/O support then we should actually
do that, not accept piecemeal patches that move us incrementally in that
direction without establishing a policy. To my mind, "establishing a
policy" would include adding a type_sanity regression test query that
shows there are no missing binary I/O functions. As of HEAD, we have

postgres=# select typname,typtype from pg_type where typreceive = 0 or typsend = 0;
typname | typtype
------------------+---------
smgr | b
aclitem | b
gtsvector | b
any | p
trigger | p
language_handler | p
internal | p
opaque | p
anyelement | p
anynonarray | p
anyenum | p
fdw_handler | p
(12 rows)

Possibly we could exclude pseudotypes from the policy, on the grounds
there are never really values of those types anyway. But other than
that, we have:

smgr: let's just get rid of that useless vestigial type.

aclitem: as per this thread, using the text representation as
the binary representation seems reasonable, or at least it doesn't
make anything any worse.

gtsvector: this is strictly an internal type, so it probably
doesn't need actual I/O support, but we could put in a couple of
dummy functions that just throw ERRCODE_FEATURE_NOT_SUPPORTED.

Maybe the right plan would be to give all the pseudotypes error-throwing
binary I/O functions too. Then, if anyone lobbies for not throwing an
error (as per what we just did with "void"), at least it doesn't take
a catversion bump to fix it.

If someone wanted to propose doing all that, I could get behind it.
But I'm not excited about debating this one datatype at a time.

regards, tom lane


From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 02:43:53
Message-ID: AANLkTimkLDRdO9S7zpcKn=tiiYK+o0LJ97zNR4Fx9gr1@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, Feb 22, 2011 at 9:26 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> Well, unfortunately, there's an awful lot of information that can only
>> be obtained in a reasonable way by introspection of the system
>> catalogs.  If you want to know whether user A can select from table B,
>> there's really no sensible way of obtaining that without parsing the
>> aclitem entries in some fashion, and unfortunately that's just the tip
>> of the iceberg.
>
> Um, that question is precisely why we invented the has_foo_privilege
> class of functions.

OK, fair point.

> I would *much* rather see users applying those
> functions than looking at ACLs directly --- and if there's some
> reasonable use-case that those don't cover, let's talk about that.

Well, the obvious applications are "I'd like to know who has
permissions on this item" and "I'd like to know what this user has
permissions to do".

>> Now, if you were to propose adding a well-designed set of DCL commands
>> to expose this kind of information to clients in a more structured
>> way, I would be the first to applaud.  LIST TABLES?  SHOW GRANTS TO?
>> Sign me up!  (I got a request for the latter just today.)  But until
>> then, if you need this information, you don't have much choice but to
>> pull it out of the system catalogs; and if JDBC would rather use
>> binary format to talk to the server, I don't particularly see any
>> reason to say "no".  If we prefer to expose the text format rather
>> than anything else, that's OK with me, but I do think it would make
>> sense to expose something.
>
> Well, to go back to the binary-format issue, if we're going to insist
> that all standard types have binary I/O support then we should actually
> do that, not accept piecemeal patches that move us incrementally in that
> direction without establishing a policy.  To my mind, "establishing a
> policy" would include adding a type_sanity regression test query that
> shows there are no missing binary I/O functions.

That's hard to disagree with. +1 for accepting a patch that fixes all
the remaining cases, but not anything less than that.

> smgr: let's just get rid of that useless vestigial type.

Heck yeah. Even if we someday want to go back to supporting more than
one smgr, the chances that we're going to want to do it this way are
just about zero.

This part probably merits its own commit, though.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


From: rsmogura <rsmogura(at)softperience(dot)eu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 09:00:00
Message-ID: 3979e7bb2aa74287d73234ff17e013b7@mail.softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>>> It'd be more future-proof than this patch, but I'm still
>>> unconvinced
>>> about the use-case.
>
>> Do we want to intentionally make binary format a second-class
>> citizen?
>
> Well, it's not exactly a first-class citizen; compare for instance
> the
> amount of verbiage in the docs about text I/O formats versus the
> amount
> about binary formats. But my question isn't about that; it's about
> why
> aclitem should be considered a first-class citizen. It makes me
> uncomfortable that client apps are looking at it at all, because any
> that do are bound to get broken in the future, even assuming that
> they
> get the right answers today. I wonder how many such clients are up
> to
> speed for per-column privileges and non-constant default privileges
> for
> instance. And sepgsql is going to cut them off at the knees.
>
> regards, tom lane

Technically, at eye glance, I didn't seen in sepgsql modifications to
acl.h. So, I think, aclitem will be unaffected. In any way sepgsql needs
some way to present access rights to administrator it may use own model,
or aclitem, too.

JDBC, and other applications may use aclitem to get just information
about who has what access. I think psql does this in same manner as
JDBC, by calling select from pg_class. But if user, through psql, JDBC
or other driver. will invoke "select * from pg_class" it will fail with
"no binary output", because it is plain user query.

Currently proposed binary output has space for 4 more privs. Am I
right?

One thing I realized, I do not pass flag if grant target is group or
user.

Regards,
Radek


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: rsmogura <rsmogura(at)softperience(dot)eu>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 15:19:27
Message-ID: 28636.1298474367@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

rsmogura <rsmogura(at)softperience(dot)eu> writes:
> On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote:
>> ... But my question isn't about that; it's about
>> why aclitem should be considered a first-class citizen. It makes me
>> uncomfortable that client apps are looking at it at all, because any
>> that do are bound to get broken in the future, even assuming that
>> they get the right answers today. I wonder how many such clients are up
>> to speed for per-column privileges and non-constant default privileges
>> for instance. And sepgsql is going to cut them off at the knees.

> Technically, at eye glance, I didn't seen in sepgsql modifications to
> acl.h. So, I think, aclitem will be unaffected. In any way sepgsql needs
> some way to present access rights to administrator it may use own model,
> or aclitem, too.

You're missing the point, which is that the current internal
representation of aclitem could change drastically to support future
feature improvements in the area of privileges. It has already changed
significantly in the past (we didn't use to have WITH GRANT OPTION).
If we had to add a field, for instance, a binary representation would
simply be broken, as clients would have difficulty telling how to
interpret it as soon as there was more than one possible format.
Text representations are typically a bit more extensible.

regards, tom lane


From: Radosław Smogura <rsmogura(at)softperience(dot)eu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 16:07:32
Message-ID: 201102231707.32587.rsmogura@softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> Wednesday 23 February 2011 16:19:27
> rsmogura <rsmogura(at)softperience(dot)eu> writes:
> > On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote:
> >> ... But my question isn't about that; it's about
> >> why aclitem should be considered a first-class citizen. It makes me
> >> uncomfortable that client apps are looking at it at all, because any
> >> that do are bound to get broken in the future, even assuming that
> >> they get the right answers today. I wonder how many such clients are up
> >> to speed for per-column privileges and non-constant default privileges
> >> for instance. And sepgsql is going to cut them off at the knees.
> >>
> > Technically, at eye glance, I didn't seen in sepgsql modifications to
> > acl.h. So, I think, aclitem will be unaffected. In any way sepgsql needs
> > some way to present access rights to administrator it may use own model,
> > or aclitem, too.
>
> You're missing the point, which is that the current internal
> representation of aclitem could change drastically to support future
> feature improvements in the area of privileges. It has already changed
> significantly in the past (we didn't use to have WITH GRANT OPTION).
> If we had to add a field, for instance, a binary representation would
> simply be broken, as clients would have difficulty telling how to
> interpret it as soon as there was more than one possible format.
> Text representations are typically a bit more extensible.
>
> regards, tom lane
I removed from patch this (think like currently not needed, but it is enaught
to put in doc)

Each privilige has idividual number P from 1 to n. and it is represented by
setted P-th bit. First n-th bits (in network bit order) represents normal
priv, next n-th bits represents grant option of privs. This "chain" is encoded
as n*2 bit number rounded up to full 8 bits, with minimal length 32 bit.

I was thinking about adding number of all privs to each ACL item, removed as
this could be deducted from PG version, where 1st 7-bit represents version,
last 8-th bit will represent if grant part has been added.

---
In any way binary output should be available, if we have binary mode. I know
that text is more extensible, we may in contrast to above "packed" version,
describes acl privs as byte array elements from represented setted priv (same
as text).

Fallback solution is to just recall aclin/aclout with StringInfo.

Regards,
Radek.


From: Radosław Smogura <rsmogura(at)softperience(dot)eu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 16:35:08
Message-ID: 201102231735.08715.rsmogura@softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> Wednesday 23 February 2011 16:19:27
> rsmogura <rsmogura(at)softperience(dot)eu> writes:
> > On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote:
> >> ... But my question isn't about that; it's about
> >> why aclitem should be considered a first-class citizen. It makes me
> >> uncomfortable that client apps are looking at it at all, because any
> >> that do are bound to get broken in the future, even assuming that
> >> they get the right answers today. I wonder how many such clients are up
> >> to speed for per-column privileges and non-constant default privileges
> >> for instance. And sepgsql is going to cut them off at the knees.
> >>
> > Technically, at eye glance, I didn't seen in sepgsql modifications to
> > acl.h. So, I think, aclitem will be unaffected. In any way sepgsql needs
> > some way to present access rights to administrator it may use own model,
> > or aclitem, too.
>
> You're missing the point, which is that the current internal
> representation of aclitem could change drastically to support future
> feature improvements in the area of privileges. It has already changed
> significantly in the past (we didn't use to have WITH GRANT OPTION).
> If we had to add a field, for instance, a binary representation would
> simply be broken, as clients would have difficulty telling how to
> interpret it as soon as there was more than one possible format.
> Text representations are typically a bit more extensible.
>
> regards, tom lane

Actully, You litlle messed in my head. So in prev post we don't need to send
information if grant option has been set, currently in text mode no grant
options means ACL_NO_RIGHTS, and in binary same may be achived be settig there
0.

But version field may be usefull to validate this and future calls, and
provide backward compatibility (if newer client will send less bits then rest
of bits will be set to 0).

I think about splitting privs chain to two numbers, it may be easier to
implement this and parse if number of privs will extend 32...

In addition I may add support for possible, future representation, where given
privilige may be yes, no, undefined (like in Windows).

Regrads,
Radek


From: Radosław Smogura <rsmogura(at)softperience(dot)eu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 18:18:22
Message-ID: 201102231918.22663.rsmogura@softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> Wednesday 23 February 2011 16:19:27
> rsmogura <rsmogura(at)softperience(dot)eu> writes:
> > On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote:
> >> ... But my question isn't about that; it's about
> >> why aclitem should be considered a first-class citizen. It makes me
> >> uncomfortable that client apps are looking at it at all, because any
> >> that do are bound to get broken in the future, even assuming that
> >> they get the right answers today. I wonder how many such clients are up
> >> to speed for per-column privileges and non-constant default privileges
> >> for instance. And sepgsql is going to cut them off at the knees.
> >>
> > Technically, at eye glance, I didn't seen in sepgsql modifications to
> > acl.h. So, I think, aclitem will be unaffected. In any way sepgsql needs
> > some way to present access rights to administrator it may use own model,
> > or aclitem, too.
>
> You're missing the point, which is that the current internal
> representation of aclitem could change drastically to support future
> feature improvements in the area of privileges. It has already changed
> significantly in the past (we didn't use to have WITH GRANT OPTION).
> If we had to add a field, for instance, a binary representation would
> simply be broken, as clients would have difficulty telling how to
> interpret it as soon as there was more than one possible format.
> Text representations are typically a bit more extensible.
>
> regards, tom lane

Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
mask, as well definition is more general then def of PGSQL. In any way it
require that rights mades bit array.

Still I tested only aclitemsend.

Btw, Is it possible and needed to add group byte, indicating that grantee is
group or user?

Regards,
Radek

Attachment Content-Type Size
aclitem_binary_20110223.patch text/x-patch 10.9 KB

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Radosław Smogura <rsmogura(at)softperience(dot)eu>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 19:48:57
Message-ID: 1298490520-sup-6326@alvh.no-ip.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Excerpts from Radosław Smogura's message of mié feb 23 15:18:22 -0300 2011:

> Btw, Is it possible and needed to add group byte, indicating that grantee is
> group or user?

There are no groups or users, only roles.

--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Radosław Smogura <rsmogura(at)softperience(dot)eu>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 21:20:54
Message-ID: 6964.1298496054@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> Excerpts from Radosaw Smogura's message of mi feb 23 15:18:22 -0300 2011:
>> Btw, Is it possible and needed to add group byte, indicating that grantee is
>> group or user?

> There are no groups or users, only roles.

Even if there were, this is not part of the value of an aclitem.

regards, tom lane


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Radosław Smogura <rsmogura(at)softperience(dot)eu>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-23 21:30:04
Message-ID: 7177.1298496604@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

=?utf-8?q?Rados=C5=82aw_Smogura?= <rsmogura(at)softperience(dot)eu> writes:
> Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
> mask, as well definition is more general then def of PGSQL. In any way it
> require that rights mades bit array.

You're going in quite the wrong direction here. The consensus as I
understood it was that we should just use the text representation in
binary mode too, rather than inventing a separate representation that's
going to put a whole new set of constraints on what can happen to the
internal representation. The proposal you have here has no redeeming
social value whatever, because nobody cares about the I/O efficiency
for aclitem (and even if anyone did, you've made no case that this would
actually be more efficient to use on the client side).

regards, tom lane


From: Merlin Moncure <mmoncure(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Radosław Smogura <rsmogura(at)softperience(dot)eu>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-24 14:38:35
Message-ID: AANLkTi=8TH7pzu45dGVEBQ-VPdgb4f48Q7GeRdt97Zip@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Wed, Feb 23, 2011 at 3:30 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> =?utf-8?q?Rados=C5=82aw_Smogura?= <rsmogura(at)softperience(dot)eu> writes:
>> Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
>> mask, as well definition is more general then def of PGSQL. In any way it
>> require that rights mades bit array.
>
> You're going in quite the wrong direction here.  The consensus as I
> understood it was that we should just use the text representation in
> binary mode too, rather than inventing a separate representation that's
> going to put a whole new set of constraints on what can happen to the
> internal representation.  The proposal you have here has no redeeming
> social value whatever, because nobody cares about the I/O efficiency
> for aclitem (and even if anyone did, you've made no case that this would
> actually be more efficient to use on the client side).

+1 on this. binary wire format is a win generally when one of the two
properties is true:

1) the receiving application is putting it into a binary structure
that is similar to what the backend sends, and conversion is
non-trivial (timestamps, geo types, etc)
2) text format needs lots of escaping (bytea, arrays etc)

Let's take the numeric type for example...if we were debating the
binary wire format for that type, I would be arguing for the backend
to send a string for the binary wire format unless someone could
present a solid case that the postgres format dropped right into a
popular numeric library in C, etc (AFAIK, it doesn't). Almost
everyone that gets a numeric will directly translate it to a string or
a hardware binary representation which the backend can't send.

Even if you could make the case for aclitem on performance grounds,
you still have to get past tom's objection (which I agree with) that
the performance benefit outweighs having to deal with making and
(especially) maintaining the binary wire format. It should be
becoming obvious to everyone the binary formats are becoming
increasingly popular, and sooner or later backwards compatibility
issues and other unresolved issues pertaining to them have to be dealt
with. Point being, let's not make that more difficult than it has to
be.

merlin


From: rsmogura <rsmogura(at)softperience(dot)eu>
To: Merlin Moncure <mmoncure(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-24 15:03:19
Message-ID: bc35db02aa3145f93750bad324e4b49d@mail.softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Thu, 24 Feb 2011 08:38:35 -0600, Merlin Moncure wrote:
> On Wed, Feb 23, 2011 at 3:30 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> =?utf-8?q?Rados=C5=82aw_Smogura?= <rsmogura(at)softperience(dot)eu> writes:
>>> Here is extended version, has version field (N_ACL_RIGHTS*2) and
>>> reserved
>>> mask, as well definition is more general then def of PGSQL. In any
>>> way it
>>> require that rights mades bit array.
>>
>> You're going in quite the wrong direction here.  The consensus as I
>> understood it was that we should just use the text representation in
>> binary mode too, rather than inventing a separate representation
>> that's
>> going to put a whole new set of constraints on what can happen to
>> the
>> internal representation.  The proposal you have here has no
>> redeeming
>> social value whatever, because nobody cares about the I/O efficiency
>> for aclitem (and even if anyone did, you've made no case that this
>> would
>> actually be more efficient to use on the client side).
>
> +1 on this. binary wire format is a win generally when one of the
> two
> properties is true:
>
> 1) the receiving application is putting it into a binary structure
> that is similar to what the backend sends, and conversion is
> non-trivial (timestamps, geo types, etc)
> 2) text format needs lots of escaping (bytea, arrays etc)

> Let's take the numeric type for example...if we were debating the
> binary wire format for that type, I would be arguing for the backend
> to send a string for the binary wire format unless someone could
> present a solid case that the postgres format dropped right into a
> popular numeric library in C, etc (AFAIK, it doesn't). Almost
> everyone that gets a numeric will directly translate it to a string
> or
> a hardware binary representation which the backend can't send.
>
> Even if you could make the case for aclitem on performance grounds,
> you still have to get past tom's objection (which I agree with) that
> the performance benefit outweighs having to deal with making and
> (especially) maintaining the binary wire format. It should be
> becoming obvious to everyone the binary formats are becoming
> increasingly popular, and sooner or later backwards compatibility
> issues and other unresolved issues pertaining to them have to be
> dealt
> with. Point being, let's not make that more difficult than it has to
> be.
>
> merlin

Thanks, but actually I didn't realized final direction, "pass to text"
or "create something really extensive", I didn't treat aclitem IO as
live or dead case, just all. I always treat performance really serious,
but I'm not psychopathic to check aclitem IO!!!

Btw, In my opinion binary format will be popular not for speed, but for
that it is internal strict, and pass in many situations more useful
informations (e.g. types for structs, arrays), it is just easier to
maintain on driver side. But it is still unpopular maybe due to missing
methods :), and few others.

Regards,
Radek


From: Radosław Smogura <rsmogura(at)softperience(dot)eu>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PG Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Binary in/out for aclitem
Date: 2011-02-24 20:02:06
Message-ID: 201102242102.07076.rsmogura@softperience.eu
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> Wednesday 23 February 2011 22:30:04
> =?utf-8?q?Rados=C5=82aw_Smogura?= <rsmogura(at)softperience(dot)eu> writes:
> > Here is extended version, has version field (N_ACL_RIGHTS*2) and reserved
> > mask, as well definition is more general then def of PGSQL. In any way it
> > require that rights mades bit array.
>
> You're going in quite the wrong direction here. The consensus as I
> understood it was that we should just use the text representation in
> binary mode too, rather than inventing a separate representation that's
> going to put a whole new set of constraints on what can happen to the
> internal representation. The proposal you have here has no redeeming
> social value whatever, because nobody cares about the I/O efficiency
> for aclitem (and even if anyone did, you've made no case that this would
> actually be more efficient to use on the client side).
>
> regards, tom lane

Look at it. Pass call to in/out.

Regards,
Radek

Attachment Content-Type Size
aclitem_binary_20110224.patch text/x-patch 8.4 KB