> On May 31, 2017, at 3:42 PM, Andres Freund <andres@anarazel.de> wrote:
>
> On 2017-05-31 15:38:58 -0700, Mark Dilger wrote:
>>> Normal users aren't going to make sense of node trees in the first
>>> place. You should use pg_get_expr for it:
>>> postgres[3008][1]=# SELECT pg_get_expr(relpartbound, oid) FROM pg_class WHERE relpartbound IS NOT NULL;
>>> ┌──────────────────────┐
>>> │ pg_get_expr │
>>> ├──────────────────────┤
>>> │ FOR VALUES IN (1, 2) │
>>> └──────────────────────┘
>>> (1 row)
>>
>> I concede that mitigates the problem somewhat, though I still think a user may look
>> at pg_class, see there is a column that appears to show the partition boundaries,
>> and then decide to check whether two tables have the same partition boundaries
>> by comparing those fields, without passing them first through pg_get_expr(), a
>> function they may never have heard of.
>>
>> To me, it seems odd to immortalize a SQL parsing field in the catalog definition of
>> the relation, but perhaps that's just my peculiar sensibilities. If the community is more
>> on your side, I'm not going to argue it.
>
> Given that various other node trees stored in the catalog also have
> location fields, about which I recall no complaints, I don't think this
> is a significant issue. Nor something that I think makes sense to solve
> in isolation, without tackling all stored expressions.
Ok. Just to clarify, I didn't come up with this problem as part of some general
code review. I merged the recent changes into my tree, and my regression
tests broke. That's fine; that's what they are there to do. Break, and in so doing,
alert me to the fact that the project has changed things that will require me to
make modifications. It just seemed strange to me that I should be changing stuff
to try to get the :location field to be equal in my code to the :location field in the
public sources, since it seems to serve no purpose.
Mark Dilger
