Bizarre choice of case for RELKIND_PARTITIONED_TABLE

On Tue, Mar 7, 2017 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?
It looks rather out of place considering that seven of the eight
pre-existing relkind codes are lower case. (And no, I don't especially
approve of RELKIND_SEQUENCE being 'S' either, but it's far too late to
change that.) Also, in typical low-res monospaced fonts, there's nearly
no difference except vertical alignment between P and p, meaning that in
something like

regression=# select distinct relkind from pg_class;
relkind
---------
r
t
P
v
m
i
S
c
(8 rows)

you have to look rather closely even to notice that what you're seeing
isn't in the case you might expect.

I think we should change this while we still can.

I can't muster a lot of outrage about this one way or another. One
possible advantage of 'P' is that there are fewer places where 'P' is
mentioned in the source code than 'p'.

[rhaas pgsql]$ git grep "'p'" | wc -l
293
[rhaas pgsql]$ git grep "'P'" | wc -l
104

...so it's a little easier to pick out the cases that are talking
about partitioned tables than it would be with a lower case letter.
However, as I say, I don't care very much.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 7, 2017 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?

I can't muster a lot of outrage about this one way or another. One
possible advantage of 'P' is that there are fewer places where 'P' is
mentioned in the source code than 'p'.

Hm, one would hope that the vast majority of code references are neither
of those, but rather "RELKIND_PARTITIONED_TABLE". information_schema.sql
and system_views.sql will need to be gone over carefully, certainly, but
we shouldn't be hard-coding this anywhere that there's a reasonable
alternative.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 3/7/17 12:55, Tom Lane wrote:

Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?

I was confused about this too. If there is no argument against it, I
would favor changing it.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Tue, Mar 7, 2017 at 6:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 7, 2017 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?

I can't muster a lot of outrage about this one way or another. One
possible advantage of 'P' is that there are fewer places where 'P' is
mentioned in the source code than 'p'.

Hm, one would hope that the vast majority of code references are neither
of those, but rather "RELKIND_PARTITIONED_TABLE". information_schema.sql
and system_views.sql will need to be gone over carefully, certainly, but
we shouldn't be hard-coding this anywhere that there's a reasonable
alternative.

For reasons which must've seemed good to whoever instituted the
policy, pg_dump refers to relkinds using the bare letters rather than
the constants.

(And protocol message types don't even have defined constants. Uggh.)

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 7, 2017 at 6:03 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Hm, one would hope that the vast majority of code references are neither
of those, but rather "RELKIND_PARTITIONED_TABLE".

For reasons which must've seemed good to whoever instituted the
policy, pg_dump refers to relkinds using the bare letters rather than
the constants.

Even in pg_dump, it appears to me that the large majority of relkind
references use the symbolic names. Quite a few of the violations of
that policy look to be new ... and now that I see them, their days are
numbered.

(And protocol message types don't even have defined constants. Uggh.)

Yeah, that's a different issue, which boils down to the fact that in order
to do anything useful we'd need to clutter client-visible namespace with
the symbols. I wouldn't be averse to doing something about it as long as
it's not done in postgres_ext.h, but if not there where?

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On 2017/03/08 2:55, Tom Lane wrote:

Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?
It looks rather out of place considering that seven of the eight
pre-existing relkind codes are lower case. (And no, I don't especially
approve of RELKIND_SEQUENCE being 'S' either, but it's far too late to
change that.) Also, in typical low-res monospaced fonts, there's nearly
no difference except vertical alignment between P and p, meaning that in
something like

regression=# select distinct relkind from pg_class;
relkind
---------
r
t
P
v
m
i
S
c
(8 rows)

you have to look rather closely even to notice that what you're seeing
isn't in the case you might expect.

I think we should change this while we still can.

I remember that one of the earliest versions of the patch I submitted had
two new relkinds: 'P' for partitioned tables and 'p' for leaf partitions.
The latter was dropped subsequently and I never thought of using 'p'
instead of 'P' for partitioned tables.

Attached patch that implements this change.

Thanks,
Amit

I wrote:

Robert Haas <robertmhaas@gmail.com> writes:

For reasons which must've seemed good to whoever instituted the
policy, pg_dump refers to relkinds using the bare letters rather than
the constants.

Even in pg_dump, it appears to me that the large majority of relkind
references use the symbolic names.

After further study, I think it was psql/describe.c you were remembering.
I cleaned that up...

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Tom Lane wrote:

(And no, I don't especially
approve of RELKIND_SEQUENCE being 'S' either, but it's far too late to
change that.)

FWIW the reason SEQUENCE uses S instead of 's' is that the latter was
taken for "special" relations, which we removed a few releases ago
(commit 3a694bb0a1).

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Alvaro Herrera <alvherre@2ndquadrant.com> writes:

Tom Lane wrote:

(And no, I don't especially
approve of RELKIND_SEQUENCE being 'S' either, but it's far too late to
change that.)

FWIW the reason SEQUENCE uses S instead of 's' is that the latter was
taken for "special" relations, which we removed a few releases ago
(commit 3a694bb0a1).

Yeah, I'd just been reminded of that by some code in describe.c.
So there actually is a reason for sequences to be 'S', or once was.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

I wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Mar 7, 2017 at 12:55 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

Is there a good reason why RELKIND_PARTITIONED_TABLE is 'P' not 'p'?

I can't muster a lot of outrage about this one way or another. One
possible advantage of 'P' is that there are fewer places where 'P' is
mentioned in the source code than 'p'.

Hm, one would hope that the vast majority of code references are neither
of those, but rather "RELKIND_PARTITIONED_TABLE". information_schema.sql
and system_views.sql will need to be gone over carefully, certainly, but
we shouldn't be hard-coding this anywhere that there's a reasonable
alternative.

Pushed. I was a bit disappointed to find that make check-world passed
just fine without having updated either information_schema.sql or
system_views.sql. Evidently our test coverage for these views leaves
something to be desired.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Thu, Mar 9, 2017 at 11:06:56PM -0300, Alvaro Herrera wrote:

Tom Lane wrote:

(And no, I don't especially
approve of RELKIND_SEQUENCE being 'S' either, but it's far too late to
change that.)

FWIW the reason SEQUENCE uses S instead of 's' is that the latter was
taken for "special" relations, which we removed a few releases ago
(commit 3a694bb0a1).

Ah! I knew there was a reason for 'S', but couldn't remember it. :-)

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers