Re: pg_class.relpartbound definition overly brittle

On 2017/06/01 11:13, Mark Dilger wrote:
>> On May 31, 2017, at 6:32 PM, Amit Langote <Langote_Amit_f8(at)lab(dot)ntt(dot)co(dot)jp> wrote:
>>
>> 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.
>
> I knew about the location field already, but not that it was already occurring
> elsewhere in the catalogs. I just never paid attention to any of the columns
> that are doing so. Alvaro's criticisms of my complaint were quite informative.
> (Thanks, Alvaro.) I think standardizing the location field to -1 at some point
> in all such places would be a good idea, though I am not sufficiently qualified
> to say if that should be in 10 or 11, nor whether doing so might cause
> backwards compatibility issues, nor whether those would be too much cost
> to justify the changes.

Setting the location field of parse nodes to -1 before writing into the
catalog for *all* node types might defeat the purpose of writing the
actual value, which as written in the header comment of readfuncs.c is:

* Parse location fields are written out by outfuncs.c, but only for
* possible debugging use. When reading a location field, we discard
* the stored value and set the location field to -1 (ie, "unknown").

So apparently, seeing the actual location value might be useful to some.
There may not be that many users who *do* use it for anything though.

Thanks,
Amit