pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Noah Misch <noah@leadboat.com>
To: Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp>
Cc: shigeru.hanada@gmail.com, Kyotaro HORIGUCHI <horiguchi.kyotaro@lab.ntt.co.jp>
Cc: pgsql-hackers@postgresql.org
Subject: Re: inherit support for foreign tables
Date: Thu, 21 Aug 2014 22:51:37 -0400
Message-ID: <20140822025137.GA563733@tornado.leadboat.com> (raw)
In-Reply-To: <53F48245.6020204@lab.ntt.co.jp>
References: <53393E18.3080901@lab.ntt.co.jp>
	<533C01C7.6030809@lab.ntt.co.jp>
	<53706FB6.6000403@lab.ntt.co.jp>
	<20140620.170406.123403564.horiguchi.kyotaro@lab.ntt.co.jp>
	<20140702022327.GC1586927@tornado.leadboat.com>
	<53F48245.6020204@lab.ntt.co.jp>
List-Unsubscribe: <mailto:majordomo@postgresql.org?body=unsub%20pgsql-hackers>

On Wed, Aug 20, 2014 at 08:11:01PM +0900, Etsuro Fujita wrote:
> (2014/07/02 11:23), Noah Misch wrote:
> >Your chosen ANALYZE behavior is fair, but the messaging from a database-wide
> >ANALYZE VERBOSE needs work:
> >
> >INFO:  analyzing "test_foreign_inherit.parent"
> >INFO:  "parent": scanned 1 of 1 pages, containing 1 live rows and 0 dead rows; 1 rows in sample, 1 estimated total rows
> >INFO:  analyzing "test_foreign_inherit.parent" inheritance tree
> >WARNING:  relcache reference leak: relation "child" not closed
> >WARNING:  relcache reference leak: relation "tchild" not closed
> >WARNING:  relcache reference leak: relation "parent" not closed
> >
> >Please arrange to omit the 'analyzing "tablename" inheritance tree' message,
> >since this ANALYZE actually skipped that task.  (The warnings obviously need a
> >fix, too.)  I do find it awkward that adding a foreign table to an inheritance
> >tree will make autovacuum stop collecting statistics on that inheritance tree,
> >but I can't think of a better policy.
> 
> I think it would be better that this is handled in the same way as
> an inheritance tree that turns out to be a singe table that doesn't
> have any descendants in acquire_inherited_sample_rows().  That would
> still output the message as shown below, but I think that that would
> be more consistent with the existing code.  The patch works so.
> (The warnings are also fixed.)
> 
> INFO:  analyzing "public.parent"
> INFO:  "parent": scanned 0 of 0 pages, containing 0 live rows and 0
> dead rows; 0 rows in sample, 0 estimated total rows
> INFO:  analyzing "public.parent" inheritance tree
> INFO:  analyzing "pg_catalog.pg_authid"
> INFO:  "pg_authid": scanned 1 of 1 pages, containing 1 live rows and
> 0 dead rows; 1 rows in sample, 1 estimated total rows

Today's ANALYZE VERBOSE messaging for former inheritance parents (tables with
relhassubclass = true but no pg_inherits.inhparent links) is deceptive, and I
welcome a fix to omit the spurious message.  As defects go, this is quite
minor.  There's fundamentally no value in collecting inheritance tree
statistics for such a parent, and no PostgreSQL command will do so.

Your proposed behavior for inheritance parents having at least one foreign
table child is more likely to mislead DBAs in practice.  An inheritance tree
genuinely exists, and a different ANALYZE command is quite capable of
collecting statistics on that inheritance tree.  Messaging should reflect the
difference between ANALYZE invocations that do such work and ANALYZE
invocations that skip it.  I'm anticipating a bug report along these lines:

  I saw poor estimates involving a child foreign table, so I ran "ANALYZE
  VERBOSE", which reported 'INFO:  analyzing "public.parent" inheritance
  tree'.  Estimates remained poor, so I scratched my head for awhile before
  noticing that pg_stats didn't report such statistics.  I then ran "ANALYZE
  VERBOSE parent", saw the same 'INFO:  analyzing "public.parent" inheritance
  tree', but this time saw relevant rows in pg_stats.  The message doesn't
  reflect the actual behavior.

I'll sympathize with that complaint.  It's a minor point overall, but the code
impact is, I predict, small enough that we may as well get it right.  A
credible alternative is to emit a second message indicating that we skipped
the inheritance tree statistics after all, and why we skipped them.


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



view thread (179+ messages)  latest in thread

Message-ID: <20140822025137.GA563733@tornado.leadboat.com>
Permalink:  ../20140822025137.GA563733@tornado.leadboat.com/
Also on:    postgresql.org/message-id/20140822025137.GA563733@tornado.leadboat.com

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-hackers@postgresql.org
  Cc: noah@leadboat.com, fujita.etsuro@lab.ntt.co.jp, horiguchi.kyotaro@lab.ntt.co.jp
  Subject: Re: inherit support for foreign tables
  In-Reply-To: <20140822025137.GA563733@tornado.leadboat.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox