From: Tom Lane <tgl@sss.pgh.pa.us>
To: James Finnerty <jfinnert@amazon.com>
cc: pgsql-hackers@lists.postgresql.org,
        Tomas Vondra <tomas.vondra@2ndquadrant.com>
Subject: Re: postgres_fdw: using TABLESAMPLE to collect remote sample
In-reply-to: 
 <167112207543.1120.12195641719862911275.pgcf@coridan.postgresql.org>
References: <151e835f-55d6-ddbc-b5b4-07ee606aba44@enterprisedb.com>
 <03182011-10a4-97b4-7c70-d99aa1c11eed@enterprisedb.com>
 <167112207543.1120.12195641719862911275.pgcf@coridan.postgresql.org>
Comments: In-reply-to James Finnerty <jfinnert@amazon.com>
	message dated "Thu, 15 Dec 2022 16:34:35 +0000"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <3377007.1671122816.1@sss.pgh.pa.us>
Date: Thu, 15 Dec 2022 11:46:56 -0500
Message-ID: <3377008.1671122816@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/3377008.1671122816%40sss.pgh.pa.us>
Precedence: bulk

James Finnerty <jfinnert@amazon.com> writes:
> This patch looks good to me.  I have two very minor nits: The inflation
> of the sample size by 10% is arbitrary but it doesn't seem unreasonable
> or concerning.  It just makes me curious if there are any known cases
> that motivated adding this logic.

I wondered why, too.

> Secondly, if the earliest non-deprecated version of PostgreSQL supports
> sampling, then you could optionally remove the logic that tests for
> that. The affected lines should be unreachable.

We've tried to keep postgres_fdw compatible with quite ancient remote
servers (I think the manual claims back to 8.3, though it's unlikely
anyone's tested that far back lately).  This patch should not move those
goalposts, especially if it's easy not to.

			regards, tom lane