Обсуждение: Binary in/out for aclitem
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
Radosław Smogura <rsmogura@softperience.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
On 02/22/2011 05:04 PM, Tom Lane wrote: > Radosław Smogura<rsmogura@softperience.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
Andrew Dunstan <andrew@dunslane.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
On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andrew Dunstan <andrew@dunslane.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
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl@sss.pgh.pa.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
[ 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@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl@sss.pgh.pa.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.
Robert Haas <robertmhaas@gmail.com> writes:
> On Tue, Feb 22, 2011 at 8:20 PM, Tom Lane <tgl@sss.pgh.pa.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 | baclitem | bgtsvector | bany | ptrigger
| planguage_handler | pinternal | popaque | panyelement | panynonarray | panyenum
| pfdw_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
On Tue, Feb 22, 2011 at 9:26 PM, Tom Lane <tgl@sss.pgh.pa.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
On Tue, 22 Feb 2011 20:20:39 -0500, Tom Lane wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Tue, Feb 22, 2011 at 5:24 PM, Tom Lane <tgl@sss.pgh.pa.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. Inany 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 insame 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
rsmogura <rsmogura@softperience.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
Tom Lane <tgl@sss.pgh.pa.us> Wednesday 23 February 2011 16:19:27 > rsmogura <rsmogura@softperience.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.
Tom Lane <tgl@sss.pgh.pa.us> Wednesday 23 February 2011 16:19:27 > rsmogura <rsmogura@softperience.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
Tom Lane <tgl@sss.pgh.pa.us> Wednesday 23 February 2011 16:19:27 > rsmogura <rsmogura@softperience.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
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@commandprompt.com> The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Alvaro Herrera <alvherre@commandprompt.com> writes:
> 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.
Even if there were, this is not part of the value of an aclitem.
regards, tom lane
Radosław Smogura <rsmogura@softperience.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
On Wed, Feb 23, 2011 at 3:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Radosław Smogura <rsmogura@softperience.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
On Thu, 24 Feb 2011 08:38:35 -0600, Merlin Moncure wrote: > On Wed, Feb 23, 2011 at 3:30 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Radosław Smogura <rsmogura@softperience.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 treataclitem IO as live or dead case, just all. I always treat performance really serious, but I'm not psychopathic to checkaclitem IO!!! Btw, In my opinion binary format will be popular not for speed, but for that it is internal strict, and pass in many situationsmore useful informations (e.g. types for structs, arrays), it is just easier to maintain on driver side. But itis still unpopular maybe due to missing methods :), and few others. Regards,Radek
Tom Lane <tgl@sss.pgh.pa.us> Wednesday 23 February 2011 22:30:04 > Radosław Smogura <rsmogura@softperience.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