Re: [HACKERS] pg_class.relpartbound definition overly brittle

> On May 31, 2017, at 4:00 PM, Alvaro Herrera <alvherre@2ndquadrant.com> wrote:
>
> Mark Dilger wrote:
>
>>>>
relpartbound                                                            |
                                                                    relpartbound
                  | ?column? | ?column? 
>>>>
----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+----------+----------
>>>> {PARTITIONBOUNDSPEC :strategy l :listdatums ({CONST :consttype 23000 :consttypmod -1 :constcollid 0 :constlen 2
:constbyvaltrue :constisnull false :location -1 :constvalue 2 [ 0 0 0 0 0 0 0 0 ]}) :lowerdatums <> :upperdatums <>
:location82} | {PARTITIONBOUNDSPEC :strategy l :listdatums ({CONST :consttype 23000 :consttypmod -1 :constcollid 0
:constlen2 :constbyval true :constisnull false :location -1 :constvalue 2 [ 0 0 0 0 0 0 0 0 ]}) :lowerdatums <>
:upperdatums<> :location 73} | f        | f       
>>>> (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.
>
> How do you expect that the used would actually interpret the above
> weird-looking value?  There's no way to figure out what operations are
> being done in that ugly blob of text.

I don't know how the average user would use it.  I can only tell you what
I am doing, which may be on the fringe of what users do typically.

I have modified the system to add a number of catalog tables not normally
present in postgres.  A few of those catalog tables are partitioned.  Since
they have to be set up at bootstrap time, I can't use the CREATE TABLE
syntax in some src/backend/catalog/*.sql file to create them, I have to
create them with header files, genbki.pl and friends.  There is no support
for this, so I created my own.  That puts me at risk of getting out of sync
with the public sources implementation of partitioning.  As such, I have
regression tests that create at run time partitioned tables that have all the
same columns and boundaries as my catalog tables, and I compare the
pg_class entries against each other to make sure there are no inconsistencies.
When you guys commit changes that impact partitioning, I notice, and change
my code to match.  But in this case, it seemed to me the change that got
committed was not thought through, and it might benefit the community for
me to point it out, rather than quietly make my code behave the same as
what got committed.

I doubt too many people will follow in my footsteps here, but other people
may hit this :location peculiarity for other reasons.

Once again, this is just to give you context about why I said anything in the
first place.

> I suspect it would be better to reduce the location field to -1 as
> proposed, though, since the location has no actual meaning once the node
> is stored standalone rather than as part of a larger command.  However
> it's clear that we're not consistent about doing this elsewhere.

I have no opinion about whether this should be done for 10.0, given the
release schedule.  Changing it for 10.0 or 11.0 seems reasonable to me.

Mark Dilger