Thread: doc - add missing documentation for "acldefault"

doc - add missing documentation for "acldefault"

From
Fabien COELHO
Date:
I couldn't find the documentation. Attached patch adds one.

Probably this function should have been named pg_*. Too late.

-- 
Fabien.
Attachment

Re: doc - add missing documentation for "acldefault"

From
Pavel Luzanov
Date:
Fabien,

On 01.08.2018 15:28, Fabien COELHO wrote:
>
> I couldn't find the documentation. Attached patch adds one.
>
> Probably this function should have been named pg_*. Too late.
>

I think this is intentionally hidden function, like others started with 
acl*.

postgres=# \df acl*
List of fun
    Schema   |    Name     | Result data type |
------------+-------------+------------------+----------------------------------
  pg_catalog | aclcontains | boolean          | aclitem[], aclitem
  pg_catalog | acldefault  | aclitem[]        | "char", oid
  pg_catalog | aclexplode  | SETOF record     | acl aclitem[], OUT 
grantor oid, O
  pg_catalog | aclinsert   | aclitem[]        | aclitem[], aclitem
  pg_catalog | aclitemeq   | boolean          | aclitem, aclitem
  pg_catalog | aclitemin   | aclitem          | cstring
  pg_catalog | aclitemout  | cstring          | aclitem
  pg_catalog | aclremove   | aclitem[]        | aclitem[], aclitem

-----
Pavel Luzanov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Re: doc - add missing documentation for "acldefault"

From
Fabien COELHO
Date:
Hello Pavel,

>> I couldn't find the documentation. Attached patch adds one.
>> 
>> Probably this function should have been named pg_*. Too late.
>
> I think this is intentionally hidden function, like others started with acl*.

Yep, I thought of that.

However, the point of having hidden and/or undocumented functions fails 
me: they are hard/impossible to find if you do not know they exist from
the start, and if you ever find one you do not know what they do without 
reading the source code in detail, eg to know what to give arguments to 
get an answer.

So I assumed that it was more lazyness and could be remedied with a doc 
patch for the one function I read. Maybe it would make sense to document 
the others as well, which are more straightforward.

Also, that does not explain why they do not use a "pg_" prefix, which is 
already a way of saying "this is an internal-purpose function". So I would 
be in favor of prefixing them with pg_: as it is an undocumented feature, 
there would not be a compatibility break, somehow:-)

> postgres=# \df acl*
> List of fun
>    Schema   |    Name     | Result data type |
> ------------+-------------+------------------+----------------------------------
>  pg_catalog | aclcontains | boolean          | aclitem[], aclitem
>  pg_catalog | acldefault  | aclitem[]        | "char", oid
>  pg_catalog | aclexplode  | SETOF record     | acl aclitem[], OUT grantor 
> oid, O
>  pg_catalog | aclinsert   | aclitem[]        | aclitem[], aclitem
>  pg_catalog | aclitemeq   | boolean          | aclitem, aclitem
>  pg_catalog | aclitemin   | aclitem          | cstring
>  pg_catalog | aclitemout  | cstring          | aclitem
>  pg_catalog | aclremove   | aclitem[]        | aclitem[], aclitem

-- 
Fabien.

Re: doc - add missing documentation for "acldefault"

From
Pavel Luzanov
Date:
Hello Fabien,

> However, the point of having hidden and/or undocumented functions 
> fails me: they are hard/impossible to find if you do not know they 
> exist from
> the start, and if you ever find one you do not know what they do 
> without reading the source code in detail, eg to know what to give 
> arguments to get an answer.
At first, we must decide in which cases users will use them.
And I don't see such cases.
I must to know how to grant privileges, how to revoke them and how to 
check existing priveleges.
All theese tasks documented in GRANT, REVOKE commands and system catalog 
descriptions.

Your's patch from another thread closes the last hole - describing 
default privileges in various psql commands.

-----
Pavel Luzanov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company



Re: doc - add missing documentation for "acldefault"

From
Arthur Zakirov
Date:
On Wed, Aug 01, 2018 at 04:41:44PM +0300, Pavel Luzanov wrote:
> postgres=# \df acl*
> List of fun
>    Schema   |    Name     | Result data type |
> ------------+-------------+------------------+----------------------------------
>  pg_catalog | aclcontains | boolean          | aclitem[], aclitem
>  pg_catalog | acldefault  | aclitem[]        | "char", oid
>  pg_catalog | aclexplode  | SETOF record     | acl aclitem[], OUT grantor
> oid, O
>  pg_catalog | aclinsert   | aclitem[]        | aclitem[], aclitem
>  pg_catalog | aclitemeq   | boolean          | aclitem, aclitem
>  pg_catalog | aclitemin   | aclitem          | cstring
>  pg_catalog | aclitemout  | cstring          | aclitem
>  pg_catalog | aclremove   | aclitem[]        | aclitem[], aclitem

Some of them are definitely internal. For example, aclitemin and
aclitemout are input and output functions respectively of the aclitem
data type. I think they don't need to be documented and shouldn't have
"pg_" prefix as they was created to maintenance aclitem data type.

-- 
Arthur Zakirov
Postgres Professional: http://www.postgrespro.com
Russian Postgres Company


Re: doc - add missing documentation for "acldefault"

From
Fabien COELHO
Date:
Hello,

>> However, the point of having hidden and/or undocumented functions fails me: 
>> they are hard/impossible to find if you do not know they exist from
>> the start, and if you ever find one you do not know what they do without 
>> reading the source code in detail, eg to know what to give arguments to get 
>> an answer.

> At first, we must decide in which cases users will use them. And I don't 
> see such cases. I must to know how to grant privileges, how to revoke 
> them and how to check existing priveleges. All theese tasks documented 
> in GRANT, REVOKE commands and system catalog descriptions.

These are end-user needs.

There are also other needs, such as devs. I see no reason to make the 
developer work harder by not providing documentation about available 
functions. Tom mention the "acldefault" function that I did not know 
existed, and I have read the doc!

So I'm still favorable to documenting all functions:-)

Maybe there could be a special section about special/internal functions, 
separate from functions which are more of interest to the end-user? But 
for me this is already the purpose of the "System information" sections in 
the documentation. Maybe there could be another sub-section about aclitem 
related functions in the "System information" section for these.

> Your's patch from another thread closes the last hole - describing default 
> privileges in various psql commands.

Yep.

-- 
Fabien.


Re: doc - add missing documentation for "acldefault"

From
Fabien COELHO
Date:
>> postgres=# \df acl*
>> List of fun
>>    Schema   |    Name     | Result data type |
>> ------------+-------------+------------------+----------------------------------
>>  pg_catalog | aclcontains | boolean          | aclitem[], aclitem
>>  pg_catalog | acldefault  | aclitem[]        | "char", oid
>>  pg_catalog | aclexplode  | SETOF record     | acl aclitem[], OUT grantor
>> oid, O
>>  pg_catalog | aclinsert   | aclitem[]        | aclitem[], aclitem
>>  pg_catalog | aclitemeq   | boolean          | aclitem, aclitem
>>  pg_catalog | aclitemin   | aclitem          | cstring
>>  pg_catalog | aclitemout  | cstring          | aclitem
>>  pg_catalog | aclremove   | aclitem[]        | aclitem[], aclitem
>
> Some of them are definitely internal. For example, aclitemin and
> aclitemout are input and output functions respectively of the aclitem
> data type. I think they don't need to be documented and shouldn't have
> "pg_" prefix as they was created to maintenance aclitem data type.

Indeed, some are internal for managing the type itself. Maybe that 
encompasses also "aclitemeq", not sure. I would see no harm in having them 
documented though.

ISTM that acl{contains,default,explode,insert,remove} deserve a real doc.

-- 
Fabien.

Re: doc - add missing documentation for "acldefault"

From
Fabien COELHO
Date:
Here is a version of the patch which documents briefly all aclitem-related 
functions, in a separate table.

-- 
Fabien.
Attachment

Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 08/03/2018 09:04 AM, Fabien COELHO wrote:
> Here is a version of the patch which documents briefly all aclitem-related
> functions, in a separate table.

I claimed this patch for review and commit. Comments:
---
* There is a comment in src/backend/utils/adt/acl.c noting that
  acldefault is "not documented for general use" which needs adjustment

* It makes no sense to me to document purely internal functions, e.g.
  aclitemin/out. If we are going to do that we should do it universally,
  which is not true currently, and IMHO makes no sense anyway.

* I do believe aclitemeq() has utility outside internal purposes.

* The <indexterm> section is incomplete

* Interestingly (since that is what started this thread apparently) I
  found myself questioning whether acldefault() is really worth
  documenting since the result will be misleading if the defaults have
  been altered via SQL.
---

Attached patch addresses those items and does a bit of reordering and
editorialization. If there are no objections I will commit it this way
in a day or two.

Thanks,

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Attachment

Re: doc - add missing documentation for "acldefault"

From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> * I do believe aclitemeq() has utility outside internal purposes.

Our normal policy is that we do not document functions that are meant to
be invoked through operators.  The \df output saying that is sufficient:

# \df+ aclitemeq
                                                                                        List of functions
   Schema   |   Name    | Result data type | Argument data types | Type | Volatility | Parallel |  Owner   | Security |
Accessprivileges | Language | Source code |         Description           

------------+-----------+------------------+---------------------+------+------------+----------+----------+----------+-------------------+----------+-------------+------------------------------
 pg_catalog | aclitemeq | boolean          | aclitem, aclitem    | func | immutable  | safe     | postgres | invoker  |
                 | internal | aclitem_eq  | implementation of = operator 
(1 row)

I would strongly object to ignoring that policy in just one place.

Actually, it appears that most of these functions have associated
operators:

# select oid::regoperator, oprcode from pg_operator where oprright = 'aclitem'::regtype;
          oid          |   oprcode
-----------------------+-------------
 +(aclitem[],aclitem)  | aclinsert
 -(aclitem[],aclitem)  | aclremove
 @>(aclitem[],aclitem) | aclcontains
 =(aclitem,aclitem)    | aclitemeq
 ~(aclitem[],aclitem)  | aclcontains
(5 rows)

So maybe what we really need is a table of operators not functions.
However, I don't object to documenting any function that has its
own pg_description string.

            regards, tom lane


Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 09/19/2018 10:54 AM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> * I do believe aclitemeq() has utility outside internal purposes.
>
> Our normal policy is that we do not document functions that are meant to
> be invoked through operators.  The \df output saying that is sufficient:

<snip>

> I would strongly object to ignoring that policy in just one place.

Ok, fair enough.

> Actually, it appears that most of these functions have associated
> operators:
>
> # select oid::regoperator, oprcode from pg_operator where oprright = 'aclitem'::regtype;
>           oid          |   oprcode
> -----------------------+-------------
>  +(aclitem[],aclitem)  | aclinsert
>  -(aclitem[],aclitem)  | aclremove
>  @>(aclitem[],aclitem) | aclcontains
>  =(aclitem,aclitem)    | aclitemeq
>  ~(aclitem[],aclitem)  | aclcontains
> (5 rows)
>
> So maybe what we really need is a table of operators not functions.

Good idea -- I will take a look at that.

> However, I don't object to documenting any function that has its
> own pg_description string.

Ok.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


Attachment

Re: doc - add missing documentation for "acldefault"

From
Fabien COELHO
Date:
Hello,

>> Our normal policy is that we do not document functions that are meant to
>> be invoked through operators.  The \df output saying that is sufficient:

The output of \df is one thing, but I was looking at pg online 
documentation and hoping to find things as well.

\dfS returns nearly 3000 results, it is not really pratical unless you 
already know what you are looking for, eg the name of the function...

>> I would strongly object to ignoring that policy in just one place.
>
> Ok, fair enough.
>
>> Actually, it appears that most of these functions have associated
>> operators:
>>
>> # select oid::regoperator, oprcode from pg_operator where oprright = 'aclitem'::regtype;
>>           oid          |   oprcode
>> -----------------------+-------------
>>  +(aclitem[],aclitem)  | aclinsert
>>  -(aclitem[],aclitem)  | aclremove
>>  @>(aclitem[],aclitem) | aclcontains
>>  =(aclitem,aclitem)    | aclitemeq
>>  ~(aclitem[],aclitem)  | aclcontains
>> (5 rows)
>>
>> So maybe what we really need is a table of operators not functions.

> Good idea -- I will take a look at that.

My initial complaint is that I did not know that there was an "acldefault" 
function while I was looking for that kind of thing. ISTM that this one 
should be kept explicitely in the doc.

I'm okay with documenting operators instead of the basic undelying 
functions and skipping type management (in/out) functions, though.

-- 
Fabien.


Re: doc - add missing documentation for "acldefault"

From
John Naylor
Date:
On 9/19/18, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> However, I don't object to documenting any function that has its
> own pg_description string.

Speaking of, that description string seems to have been neglected.
I've attached a remedy for that.

-John Naylor

Attachment

Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 09/19/2018 12:30 PM, John Naylor wrote:
> On 9/19/18, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> However, I don't object to documenting any function that has its
>> own pg_description string.
>
> Speaking of, that description string seems to have been neglected.
> I've attached a remedy for that.

Thanks, will take care of it.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


Attachment

Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 09/19/2018 11:18 AM, Joe Conway wrote:
> On 09/19/2018 10:54 AM, Tom Lane wrote:
>> Joe Conway <mail@joeconway.com> writes:
>>> * I do believe aclitemeq() has utility outside internal purposes.
>>
>> Our normal policy is that we do not document functions that are meant to
>> be invoked through operators.  The \df output saying that is sufficient:
>
> <snip>
>
>> I would strongly object to ignoring that policy in just one place.
>
> Ok, fair enough.
>
>> Actually, it appears that most of these functions have associated
>> operators:
>>
>> # select oid::regoperator, oprcode from pg_operator where oprright = 'aclitem'::regtype;
>>           oid          |   oprcode
>> -----------------------+-------------
>>  +(aclitem[],aclitem)  | aclinsert
>>  -(aclitem[],aclitem)  | aclremove
>>  @>(aclitem[],aclitem) | aclcontains
>>  =(aclitem,aclitem)    | aclitemeq
>>  ~(aclitem[],aclitem)  | aclcontains
>> (5 rows)
>>
>> So maybe what we really need is a table of operators not functions.
>
> Good idea -- I will take a look at that.
>
>> However, I don't object to documenting any function that has its
>> own pg_description string.

Ok, so the attached version refactors/splits the group into two tables
-- operators and functions.

It drops aclinsert and aclremove entirely due to the fact that they no
longer do anything useful, to wit:
-----
Datum
aclinsert(PG_FUNCTION_ARGS)
{
    ereport(ERROR,
        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
         errmsg("aclinsert is no longer supported")));

    PG_RETURN_NULL();            /* keep compiler quiet */
}

Datum
aclremove(PG_FUNCTION_ARGS)
{
    ereport(ERROR,
        (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
         errmsg("aclremove is no longer supported")));

    PG_RETURN_NULL();            /* keep compiler quiet */
}
-----

I also included John Naylor's patch with some minor editorialization.

Any further comments or complaints?

Thanks,

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

Attachment

Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 09/21/2018 01:51 PM, Joe Conway wrote:
> On 09/19/2018 11:18 AM, Joe Conway wrote:
>> On 09/19/2018 10:54 AM, Tom Lane wrote:
>>> So maybe what we really need is a table of operators not functions.
>>
>> Good idea -- I will take a look at that.
>>
>>> However, I don't object to documenting any function that has its
>>> own pg_description string.
>
> Ok, so the attached version refactors/splits the group into two tables
> -- operators and functions.

<snip>

> I also included John Naylor's patch with some minor editorialization.
>
> Any further comments or complaints?

Having seen none, committed/pushed. This did not seem worth
back-patching, so I only pushed to master.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


Attachment

Re: doc - add missing documentation for "acldefault"

From
Tom Lane
Date:
Joe Conway <mail@joeconway.com> writes:
> Having seen none, committed/pushed. This did not seem worth
> back-patching, so I only pushed to master.

I don't see anything on gitmaster?

            regards, tom lane


Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 09/24/2018 10:01 AM, Tom Lane wrote:
> Joe Conway <mail@joeconway.com> writes:
>> Having seen none, committed/pushed. This did not seem worth
>> back-patching, so I only pushed to master.
>
> I don't see anything on gitmaster?

Hmm, yes, interesting -- I must of messed up my local git repo somehow.
Will try again.

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


Attachment

Re: doc - add missing documentation for "acldefault"

From
Joe Conway
Date:
On 09/24/2018 10:09 AM, Joe Conway wrote:
> On 09/24/2018 10:01 AM, Tom Lane wrote:
>> Joe Conway <mail@joeconway.com> writes:
>>> Having seen none, committed/pushed. This did not seem worth
>>> back-patching, so I only pushed to master.
>>
>> I don't see anything on gitmaster?
>
> Hmm, yes, interesting -- I must of messed up my local git repo somehow.
> Will try again.

This time it seems to have worked. Sorry for the noise earlier :-/

Joe

--
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development


Attachment