Re: inherit support for foreign tables

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.