On 2025-07-15 12:31, jian he wrote:
On Mon, Jul 14, 2025 at 10:02 PM Álvaro Herrera <alvhe...@kurilemu.de>
wrote:
On 2025-Jul-02, jian he wrote:
> @@ -673,11 +680,34 @@ BeginCopyTo(ParseState *pstate,
> errmsg("cannot copy from sequence
\"%s\"",
>
RelationGetRelationName(rel))));
> else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
> - ereport(ERROR,
> - (errcode(ERRCODE_WRONG_OBJECT_TYPE),
> - errmsg("cannot copy from partitioned table
\"%s\"",
> -
RelationGetRelationName(rel)),
> - errhint("Try the COPY (SELECT ...) TO
variant.")));
> + {
> + children = find_all_inheritors(RelationGetRelid(rel),
> +
AccessShareLock,
> +
NULL);
> +
> + foreach_oid(childreloid, children)
> + {
> + char relkind =
get_rel_relkind(childreloid);
> +
> + if (relkind == RELKIND_FOREIGN_TABLE)
> + {
> + char *relation_name;
> +
> + relation_name =
get_rel_name(childreloid);
> + ereport(ERROR,
> +
errcode(ERRCODE_WRONG_OBJECT_TYPE),
> + errmsg("cannot copy from foreign table
\"%s\"", relation_name),
> + errdetail("Partition \"%s\" is a
foreign table in the partitioned table \"%s\"",
> +
relation_name, RelationGetRelationName(rel)),
> + errhint("Try the COPY
(SELECT ...) TO variant."));
> + }
This code looks like it's duplicating what you could obtain by using
RelationGetPartitionDesc and then observe the ->isleaf bits. Maybe
have
a look at the function RelationHasForeignPartition() in the patch at
https://postgr.es/m/canhcyew_s2ld6ridsmhtwqnpyb67ewxqf7n8mn7dornakmf...@mail.gmail.com
which looks very similar to what you need here. I think that would
also
have the (maybe dubious) advantage that the rows will be output in
partition bound order rather than breadth-first (partition hierarchy)
OID order.
hi.
else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
{
PartitionDesc pd = RelationGetPartitionDesc(rel, true);
for (int i = 0; i < pd->nparts; i++)
{
Relation partRel;
if (!pd->is_leaf[i])
continue;
partRel = table_open(pd->oids[i], AccessShareLock);
if (partRel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
errcode(ERRCODE_WRONG_OBJECT_TYPE),
errmsg("cannot copy from foreign table
\"%s\"", RelationGetRelationName(partRel)),
errdetail("Partition \"%s\" is a foreign
table in the partitioned table \"%s\"",
RelationGetRelationName(partRel), RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO
variant."));
table_close(partRel, NoLock);
scan_oids = lappend_oid(scan_oids,
RelationGetRelid(partRel));
}
}
I tried the above code, but it doesn't work because
RelationGetPartitionDesc
only retrieves the immediate partition descriptor of a partitioned
relation, it
doesn't recurse to the lowest level.
Actually Melih Mutlu raised this question at
https://postgr.es/m/CAGPVpCQou3hWQYUqXNTLKdcuO6envsWJYSJqbZZQnRCjZA6nkQ%40mail.gmail.com
I kind of ignored it...
I guess we have to stick with find_all_inheritors here?
That might be the case.
I thought we could consider using RelationHasForeignPartition() instead,
if [1] gets committed.
However, since that function only tells us whether any foreign
partitions exist, whereas the current patch outputs the specific
problematic partitions or foreign tables in the log, I think the current
approach is more user-friendly.
> <command>COPY TO</command> can be used with plain
> - tables and populated materialized views.
> - For example,
> + tables, populated materialized views and partitioned tables.
> + For example, if <replaceable
class="parameter">table</replaceable> is not partitioned table,
> <literal>COPY <replaceable class="parameter">table</replaceable>
> TO</literal> copies the same rows as
> <literal>SELECT * FROM ONLY <replaceable
class="parameter">table</replaceable></literal>.
I believe "is not a partitioned table" here is intended to refer to both
plain tables and materialized views.
However, as far as I understand, using ONLY with a materialized view has
no effect.
So, wouldn’t it be better and clearer to say "if the table is a plain
table" instead?
I think the behavior for materialized views can be described along with
that for partitioned tables. For example:
<command>COPY TO</command> can be used with plain
tables, populated materialized views and partitioned tables.
For example, if <replaceable class="parameter">table</replaceable>
is a plain table,
<literal>COPY <replaceable class="parameter">table</replaceable>
TO</literal> copies the same rows as
<literal>SELECT * FROM ONLY <replaceable
class="parameter">table</replaceable></literal>.
If <replaceable class="parameter">table</replaceable> is a
partitioned table or a materialized view,
<literal>COPY <replaceable class="parameter">table</replaceable>
TO</literal>
copies the same rows as <literal>SELECT * FROM <replaceable
class="parameter">table</replaceable></literal>.
+ List *children = NIL;
...
@@ -673,11 +680,34 @@ BeginCopyTo(ParseState *pstate,
errmsg("cannot copy from sequence \"%s\"",
RelationGetRelationName(rel))));
else if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from partitioned table
\"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO
variant.")));
+ {
+ children = find_all_inheritors(RelationGetRelid(rel),
Since 'children' is only used inside the else if block, I think we don't
need the separate "List *children = NIL;" declaration.
Instead, it could just be "List *children = find_all_inheritors(...)".
[1]
https://www.postgresql.org/message-id/flat/CANhcyEW_s2LD6RiDSMHtWQnpYB67EWXqf7N8mn7dOrnaKMfROg%40mail.gmail.com#7bfbe70576e7a3f7162ca268f08bd395
--
Regards,
--
Atsushi Torikoshi
Seconded from NTT DATA Japan Corporation to SRA OSS K.K.
