Re: Should pg_current_wal_location() become pg_current_wal_lsn()

At Fri, 14 Apr 2017 18:26:37 -0400, Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote in <052f4ce0-159a-1666-f136-91977d3267a5(at)2ndquadrant(dot)com>
> On 4/14/17 04:28, Kyotaro HORIGUCHI wrote:
> > =# select distinct attname from pg_attribute where attname like '%lsn%';
> > attname
> > ---------------------
> > confirmed_flush_lsn
> > latest_end_lsn
> > local_lsn
> > receive_start_lsn
> > received_lsn
> > remote_lsn
> > restart_lsn
> > srsublsn
> > (8 rows)
> >
> >
> > Feature is already frozen, but this seems inconsistent a bit..
>
> I think these are all recently added for logical replication. We could
> rename them to _location.
>
> I'm not a fan of renaming everything the opposite way.

I don't particulary care for either. What is most unpleasant here
for me is the inconsistency among several replication-related
tables. Logical replication stuff is using LSN and physical sutff
has been using location, but pg_stat_wal_receiver is using
LSN. pg_replication_slots as the common stuff is using LSN.

"Location" fits attribute names since the table name implies that
the location is "LSN".

On the other hand nothing suggests such implication on function
names. So only "wal_location" or "lsn" can be used in function
names. pg_current_wal_* requires to be "wal_lsn" even using LSN
since "LSN" itself doesn't imply WAL files being
written. "wal_lsn" looks somewhat too-much, though.

Columns:
=# select attrelid::regclass || '.' || attname from pg_attribute where attname like '%location%' or attname like '%lsn%';
?column?
------------------------------------------
pg_subscription_rel.srsublsn
pg_stat_replication.sent_location
pg_stat_replication.write_location
pg_stat_replication.flush_location
pg_stat_replication.replay_location
pg_stat_wal_receiver.receive_start_lsn
pg_stat_wal_receiver.received_lsn
pg_stat_wal_receiver.latest_end_lsn
pg_stat_subscription.received_lsn
pg_stat_subscription.latest_end_lsn
pg_replication_slots.restart_lsn
pg_replication_slots.confirmed_flush_lsn
pg_replication_origin_status.remote_lsn
pg_replication_origin_status.local_lsn

pg_subscription_rel has a bit different naming convention from
others. But I'm not sure that involving it in the unification is
good since it doesn't seem to be explicitly exposed to users.

=# select proname from pg_proc where proname like '%location%' or proname like '%lsn%';
proname
--------------------------------
pg_tablespace_location ## This is irrelevant
pg_current_wal_location
pg_current_wal_insert_location
pg_current_wal_flush_location
pg_wal_location_diff
pg_last_wal_receive_location
pg_last_wal_replay_location
pg_lsn_mi
pg_lsn_in
pg_lsn_out
pg_lsn_lt
pg_lsn_le
pg_lsn_eq
pg_lsn_ge
pg_lsn_gt
pg_lsn_ne
pg_lsn_recv
pg_lsn_send
pg_lsn_cmp
pg_lsn_hash

I think we can use "location" for all attributes and functions
except pg_lsn operators.

The last annoyance would be pg_wal_location_diff(). This exists
only for backward compatibility but the name 'pg_wal_lsn_diff' is
already so far from the original name that it becomes totally
useless.

Any more thoughts?

regards,

--
Kyotaro Horiguchi
NTT Open Source Software Center