Feedback-ID: i0fe9450f:Fastmail
Date: Tue, 18 Feb 2025 09:49:22 +0900
From: Michael Paquier <michael@paquier.xyz>
To: Zhang Mingli <zmlpostgres@gmail.com>
Cc: Sami Imseih <samimseih@gmail.com>,
	=?iso-8859-1?Q?=C1lvaro?= Herrera <alvherre@alvh.no-ip.org>,
	PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Proposal to CREATE FOREIGN TABLE LIKE
Message-ID: <Z7PZEvGU9KFQ2Qkh@paquier.xyz>
References: <9324b4d3-d49f-463e-984a-2725fe36cb2b@Spark>
 <3ded6d8c-c417-4812-8861-12cbb7f8e182@Spark>
 <c5dc00e9-3f19-47fc-8409-47581020d09e@Spark>
 <CAA5RZ0uR9Km4qB9JMhbyi2iMc3E0PdCRz=GWn==_5t=Q2Sxosw@mail.gmail.com>
 <3904c689-2801-4916-99ed-7a7e1d86da3e@Spark>
 <CAA5RZ0ts9vBOQ1=igQfzjYT5Df6Lt=hHn=9wD13PxWMsu18dfQ@mail.gmail.com>
 <532c4759-52d4-46c9-8eb2-6a836e3ece22@Spark>
 <CAA5RZ0vsbWeNo9MT7xT_3vaDLJU7Uumn4HUU3aqUVbZ7G7YK4w@mail.gmail.com>
 <Z7LkDITAw5e79X_k@paquier.xyz>
 <ae691413-72c5-4f17-8a31-141f70bbabd6@Spark>
MIME-Version: 1.0
Content-Type: multipart/signed; micalg=pgp-sha512;
	protocol="application/pgp-signature"; boundary="Nvjg+FFtaF8e722I"
Content-Disposition: inline
In-Reply-To: <ae691413-72c5-4f17-8a31-141f70bbabd6@Spark>
Archived-At: <https://www.postgresql.org/message-id/Z7PZEvGU9KFQ2Qkh%40paquier.xyz>
Precedence: bulk


--Nvjg+FFtaF8e722I
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable

On Mon, Feb 17, 2025 at 07:14:59PM +0800, Zhang Mingli wrote:
> On Feb 17, 2025 at 15:24 +0800, Michael Paquier <michael@paquier.xyz>, wr=
ote:
>> + * For foreign tables, they have no storage in Postgres.
>> + * Inapplicable options are ignored.
>>
>> Wording is a bit strange here.
>=20
> =C2=A0* Foreign tables do not store data in Postgres.
> =C2=A0* Any options that are not applicable for foreign tables will be ig=
nored:

I would do something like that, perhaps, though I could get that
people don't like this suggestion:
"Some options are ignored.  For example, as foreign tables have no
storage, these options have no effect: storage, compression, identity
and indexes.  Similarly, INCLUDING INDEXES is ignored from a view."

> I usually follow the cases within the same file; for each
> independent option, it becomes easier to identify which options are
> valid or invalid.  However, if you believe consolidating them into
> one is better, I=E2=80=99m fine with that. Updated.

It does not matter much in this case, IMO, what matters is to make
sure that the four additional checks you are adding in the
post-parsing paths are correctly ignored.  Passing down an INCLUDING
ALL does that just fine as you double-check the state of the table
with a \d+ meta-command.

> I also didn't realize this until I wrote this patch. This could be
> useful for the planner=EF=BC=9F

Constraints can be used as hints in the planner when working on
foreign tables.  I'm pretty sure that this is the same reason here,
seeing that this is supported since v10 where statistics have been
introduced.  I would need to dig more into the code, but that's not
really the point for this thread..

>> expandTableLikeClause() depends on the "constr" built in the first
>> phase of transformTableLikeClause() to bypass the parts related to
>> generated, default and constraints properties. This dependency
>> between both routines should be documented, I guess. Hmm.. Not sure
>> where.
>
> Comments are addressed except for this one, it might be worth
> considering another patch.

Yeah, not entirely sure.  It does not prevent your patch to work, and
it is already documented that the "expand" part is done as-is because
it depends on the "transform".  So perhaps it's just OK like this.  I
withdraw my comment.

+      Inapplicable options: <literal>INCLUDING INDEXES</literal>, <literal=
>INCLUDING STORAGE</literal>,
+      <literal>INCLUDING COMPRESSION</literal>, <literal>INCLUDING IDENTIT=
Y</literal> are ignored.

I would remove this paragraph, actually.  The options supported are
listed by your patch, and that would be one area less to update if a
new INCLUDING flavor is added.

Copy-pasting the details of how the LIKE options work to the
create_foreign_table.sgml page is OK for me, and perhaps this will
diverge a bit from the CREATE TABLE part.  One thing is that LIKE is
not part of the SQL specification for CREATE FOREIGN TABLE.  Perhaps
this should be mentioned at the bottom of the page under the
"compatibility" section?
--
Michael

--Nvjg+FFtaF8e722I
Content-Type: application/pgp-signature; name="signature.asc"

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEEG72nH6vTowiyblFKnvQgOdbyQH0FAmez2RIACgkQnvQgOdby
QH2Yqg/+JNsIZAMcO/Hzie8ZBATzo0lQDXxciP2t3E7vn2tSk9gckKvk/weLaYae
xzbtLWXIwXCBSk+drxHDdDzZh9FYMW/JotZIXxB5aKeEOAm4NHTO7rWLzSTWSCyQ
0uJ6PW04LtlFKnAa4K5AuhAU1xIHERHXQV2BN6h79bOzAr/BzpLrTGPKqOX1Xe6r
TKUnLpHYzerNyv6LbBJsd7W+SdDoAMO2+1mIEindlnbOd7wnAR1CPvbHS/E1RzlS
w1q1dKZ8XzlugSgv2f9BdSH3iQmXq62UeSpFIGbV3A6PPfaonCjr4u/LNoPv/o8M
c3iN43Euurn0mM+F5hxm7H8nZTeyWHEFp44RiAKh5yhBqe4fCjf+LidBhJRsG4Em
anhnAv9i74H7zEsRvoSwjqES+Z99fZoxa7MZZ2o7U5oFUBkHhVM4LZZuE40ZZYMM
0kR4gVWiKspOoOziseTIzJdKHYRlvxfuj3xPszIQyAC7DyC3fmJ26xN25G+d5wwe
Sv5Qn+dKq5Ue0Mr4HRtCmWfGsJS+oNtybXq6HgPiTyDs5gDiHfkm4SGEp2ns6V1w
Uqn0NNHGl+o0t0Hh66ULG4VHdTToMRaHhuoBhUaR2omd0hvqdf6RXCEgAy0lHdOz
eQDWljRzm206vca9fBpg+bKXG4TiraHGb/+f9WQ0swa+gx6BcS4=
=Gdzo
-----END PGP SIGNATURE-----

--Nvjg+FFtaF8e722I--