On 2025/04/03 17:53, jian he wrote:
> On Wed, Apr 2, 2025 at 11:20 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
>>>
>>> if (!RelationIsPopulated(rel))
>>> ereport(ERROR,
>>> errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
>>> errmsg("cannot copy from unpopulated
>>> materialized view \"%s\"",
>>> RelationGetRelationName(rel)),
>>> errhint("Use the REFRESH MATERIALIZED VIEW
>>> command to populate the materialized view first."));
>>
>> I think it's better to use the same hint message as the one output by
>> "COPY (SELECT * FROM <unpopulated matview>) TO",
>> specifically: "Use the REFRESH MATERIALIZED VIEW command," for consistency.
>>
> ok.
>
>>
>>>> The copy.sgml documentation should clarify that COPY TO can
>>>> be used with a materialized view only if it is populated.
>>>>
>>> "COPY TO can be used only with plain tables, not views, and does not
>>> copy rows from child tables or child partitions"
>>> i changed it to
>>> "COPY TO can be used with plain tables and materialized views, not
>>> regular views, and does not copy rows from child tables or child
>>> partitions"
>>
>> It would be clearer to specify that "COPY TO" applies to *populated*
>> materialized views rather than just "materialized views"?
>>
>>
>>> Another alternative wording I came up with:
>>> "COPY TO can only be used with plain tables and materialized views,
>>> not regular views. It also does not copy rows from child tables or
>>> child partitions."
>>
>> If we split the first description into two as you suggested,
>> I'm tempted to propose the following improvements to enhance
>> the overall descriptions:
>>
>> -------------
>> "COPY TO" can be used with plain tables and populated materialized views. For example, "COPY table TO" copies the
samerows as "SELECT * FROM ONLY table." However, it doesn't directly support other relation types, such as partitioned
tables,inheritance child tables, or views. To copy all rows from these relations, use "COPY (SELECT * FROM table) TO."
>> -------------
>>
>
> your wording makes sense to me.
> I try to ensure that the changing part in copy.sgml the line width
> is less than 80 characters.
> but I also want to make sure "<>" "</>" within the same line.
> so after the change it becomes:
>
> <para>
> <command>COPY TO</command> can be used with plain
> tables and populated materialized views.
> For example,
> <literal>COPY <replaceable class="parameter">table</replaceable>
> TO</literal>
> copies the same rows as
> <literal>SELECT * FROM ONLY <replaceable
> class="parameter">table</replaceable></literal>.
> However it doesn't directly support other relation types,
> such as partitioned tables, inheritance child tables, or views.
>
>
>
>> The tests seem to have been placed under the category "COPY FROM ... DEFAULT",
>> which feels a bit misaligned. How about adding them to the end of copy.sql instead?
>>
> ok.
Thanks for updating the patch! I made some minor cosmetic changes
and updated the commit log. The revised patch is attached.
Unless there are any objections, I'll proceed with committing it.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
