Re: On the warpath again about ill-considered inclusion nests

Stephen Frost <sfrost(at)snowman(dot)net> writes:
> * Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)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.