Re: pg_class.relpartbound definition overly brittle

On 2017/06/01 4:50, Robert Haas wrote:
> On Wed, May 31, 2017 at 3:40 PM, Mark Dilger <hornschnorter(at)gmail(dot)com> wrote:
>> recent changes have introduced the :location field to the partboundspec
>> in pg_catalog.pg_class. This means that if two identical tables with
>> identical partitioning scheme are created, but one is done before a change
>> to gram.y, and the other after a change to gram.y, the relpartbound fields
>> for those two tables could show up as different.
>>
>> Can we perhaps remove the :location field here? If not, could somebody
>> please defend why this belongs in the catalog entries for the table? Sorry
>> if I am missing something obvious...

I now think it's kind of annoying too, although not exactly unprecedented
as others have pointed out. As you well might know, the location field in
the parse node is to position the error cursor at the correct point in the
erring command text. Although, the node structure that helps do that and
one that we need to store into the catalog do not have to be same, as
Robert suggests in his reply. Doing that division will again have to
break the catalog though.

By the way, didn't you first have to come across that? The commit
(80f583ffe93) that introduced location field to partboundspec also bumped
up the catalog version, so you would have to reinitialize the database
directory anyway.

> Yeah, that probably wasn't a good decision, although calling a
> decision might be giving it too much credit. I think the easiest
> thing to do here would be to change transformPartitionBound() to set
> result_spec->location to -1, although maybe a better idea would be to
> have two different structures -- one that represents the partition
> bound specification *before* parse analysis and another that
> represents it *after* parse analysis, rather than reusing the same
> structure for both. Then again, maybe making two different node types
> for this is overkill. Not sure. Opinions?

I find the two node structures idea interesting, though it would require
breaking the catalog again.

Thanks,
Amit