Re: On the warpath again about ill-considered inclusion nests - Mailing list pgsql-hackers

From Tom Lane
Subject Re: On the warpath again about ill-considered inclusion nests
Date
Msg-id 9955.1415892770@sss.pgh.pa.us
Whole thread Raw
In response to Re: On the warpath again about ill-considered inclusion nests  (Stephen Frost <sfrost@snowman.net>)
Responses Re: On the warpath again about ill-considered inclusion nests  (Stephen Frost <sfrost@snowman.net>)
List pgsql-hackers
Stephen Frost <sfrost@snowman.net> writes:
> * Tom Lane (tgl@sss.pgh.pa.us) wrote:
>> has any business including.  And if that weren't bad enough, it's
>> been included into utils/rel.h (relcache),

> This is for the definition of RowSecurityDesc.  I'm happy to move that
> to a utils/rowsecurity.h instead, following how TriggerDesc is handled.

Well, if you *only* move RowSecurityDesc and not RowSecurityPolicy,
okay, but that seems a bit useless/inconsistent if I'm reading it
right that RowSecurityDesc contains a List of RowSecurityPolicy structs.
If you move both, I'm still gonna be on your case because rel.h has no
business depending on parsenodes.h or array.h.

What seems possibly saner is to just remove the header inclusion in rel.h
and declare the new Relation field similarly to the way we handle
rd_fdwroutine and some other fields there:
/* use "struct" here to avoid needing to include rowsecurity.h: */struct RowSecurityDesc *rsdesc;    /* Row-security
policy,or NULL */
 

And while you are at it, how about renaming "rsdesc" to "rd_rsdesc"?
The fact that whoever put in trigdesc didn't get the memo about the
naming convention for Relation fields doesn't excuse you from following
it.
        regards, tom lane

PS: The comments for struct RowSecurityPolicy could stand to be improved.



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: WAL format and API changes (9.5)
Next
From: Katharina Büchse
Date:
Subject: Re: WIP: multivariate statistics / proof of concept