Date: Tue, 13 Aug 2019 10:01:27 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Tatsuro Yamada <tatsuro.yamada.tf@nttcom.co.jp>
Cc: Pg Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: progress report for ANALYZE
Message-ID: <20190813140127.GA4933@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <45c2a6c0-80a1-0c55-b7d8-4ee15fd17841@nttcom.co.jp>
User-Agent: Mutt/1.9.4 (2018-02-28)
Precedence: bulk

Hello,

On 2019-Jul-03, Tatsuro Yamada wrote:

> My ex-colleague Vinayak created same patch in 2017 [1], and he
> couldn't get commit because there are some reasons such as the
> patch couldn't handle analyzing Foreign table. Therefore, I wonder
> whether your patch is able to do that or not.

> [1] ANALYZE command progress checker
> https://www.postgresql.org/message-id/flat/968b4eda-2417-8b7b-d468-71643cf088b6%40openscg.com#574488592fcc9708c38fa44b0dae9006

So just now I went to check the jold thread (which I should have
searched for before writing my own implementation!).  It seems clear
that many things are pretty similar in both patch, but I think for the
most part they are similar just because the underlying infrastructure
imposes a certain design already, rather than there being any actual
copying.  (To be perfectly clear: I didn't even know that this patch
existed and I didn't grab any code from there to produce my v1.)

However, you've now modified the patch from what I submitted and I'm
wondering if you've taken any inspiration from Vinayak's old patch.  If
so, it seems fair to credit him as co-author in the commit message.  It
would be good to get his input on the current patch, though.

I have not looked at the current version of the patch yet, but I intend
to do so during the upcoming commitfest.

Thanks for moving this forward!


On the subject of FDW support: I did look into supporting that before
submitting this.  I think it's not academically difficult: just have the
FDW's acquire_sample_rows callback invoke the update_param functions
once in a while.  Sadly, in practical terms it looks like postgres_fdw
is quite stupid about ANALYZE (it scans the whole table??) so doing
something that's actually useful may not be so easy.  At least, we know
the total relation size and maybe we can add the ctid column to the
cursor in postgresAcquireSampleRowsFunc so that we have a current block
number to report (becing careful about synchronized seqscans).  I think
this should *not* be part of the main ANALYZE-progress commit, though,
because getting that properly sorted out is going to take some more
time.

I do wonder why doesn't postgres_fdw use TABLESAMPLE.

I did not look at other FDWs at all, mind.

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