Re: [HACKERS] Should pg_current_wal_location() becomepg_current_wal_lsn() - Mailing list pgsql-hackers

From Kyotaro HORIGUCHI
Subject Re: [HACKERS] Should pg_current_wal_location() becomepg_current_wal_lsn()
Date
Msg-id 20170417.143937.232025253.horiguchi.kyotaro@lab.ntt.co.jp
Whole thread Raw
In response to Re: [HACKERS] Should pg_current_wal_location() becomepg_current_wal_lsn()  (Peter Eisentraut <peter.eisentraut@2ndquadrant.com>)
List pgsql-hackers
At Fri, 14 Apr 2017 18:26:37 -0400, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote in
<052f4ce0-159a-1666-f136-91977d3267a5@2ndquadrant.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.srsublsnpg_stat_replication.sent_locationpg_stat_replication.write_locationpg_stat_replication.flush_locationpg_stat_replication.replay_locationpg_stat_wal_receiver.receive_start_lsnpg_stat_wal_receiver.received_lsnpg_stat_wal_receiver.latest_end_lsnpg_stat_subscription.received_lsnpg_stat_subscription.latest_end_lsnpg_replication_slots.restart_lsnpg_replication_slots.confirmed_flush_lsnpg_replication_origin_status.remote_lsnpg_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
irrelevantpg_current_wal_locationpg_current_wal_insert_locationpg_current_wal_flush_locationpg_wal_location_diffpg_last_wal_receive_locationpg_last_wal_replay_locationpg_lsn_mipg_lsn_inpg_lsn_outpg_lsn_ltpg_lsn_lepg_lsn_eqpg_lsn_gepg_lsn_gtpg_lsn_nepg_lsn_recvpg_lsn_sendpg_lsn_cmppg_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




pgsql-hackers by date:

Previous
From: Robert Haas
Date:
Subject: Re: [HACKERS] Inadequate parallel-safety check for SubPlans
Next
From: Robert Haas
Date:
Subject: Re: [HACKERS] Shouldn't duplicate addition to publication be a no-op?