On Feb 17, 2025 at 15:24 +0800, Michael Paquier <mich...@paquier.xyz>, wrote: > > + * For foreign tables, they have no storage in Postgres. > + * Inapplicable options are ignored. > > Wording is a bit strange here. Hi, is this better?
* Foreign tables do not store data in Postgres. * Any options that are not applicable for foreign tables will be ignored: > + * CREATE_TABLE_LIKE_COMPRESSION > + * CREATE_TABLE_LIKE_IDENTITY > + * CREATE_TABLE_LIKE_STORAGE > + * CREATE_TABLE_LIKE_INDEXES > [...] > +DROP FOREIGN TABLE ctl_foreign_table1; > +DROP FOREIGN TABLE ctl_foreign_table2; > +DROP FOREIGN TABLE ctl_foreign_table3; > +DROP FOREIGN TABLE ctl_foreign_table4; > +DROP FOREIGN TABLE ctl_foreign_table5; > +DROP FOREIGN TABLE ctl_foreign_table6; > > What's the point of creating that many tables, one for each of the > four INCLUDING options you are testing? It is possible to stack all > of them in a single CREATE TABLE command, still is that really > necessary if we have coverage with INCLUDING ALL and EXCLUDING ALL > (perhaps write it directly in the CREATE query rather than assume that > it is the default) as these are all-or-nothing switches for all the > option values. Of course let's be careful with HIDE_TOAST_COMPRESSION > if need be. 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’m fine with that. Updated. > Perhaps the original table should have a primary key, also to track > the fact that the NOT NULL constraint is always copied but its related > index is not? Identity columns assign a NOT NULL constraint, as > documented these are always copied. Just wanted to point out that > this works the same way for your implementation with foreign tables, > so perhaps we should have a test for that. Done, although there is already one in column d. > > Glad to see that you did not forget about statistics. I didn't recall > that these were OK for foreign tables, TBH. I also didn't realize this until I wrote this patch. This could be useful for the planner？ > 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. -- Zhang Mingli HashData
v5-0001-CREATE-FOREIGN-TABLE-LIKE.patch
Description: Binary data
