public inbox for [email protected]
help / color / mirror / Atom feedRe: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
28+ messages / 8 participants
[nested] [flat]
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2024-12-20 02:53 jian he <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: jian he @ 2024-12-20 02:53 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Fri, Dec 20, 2024 at 8:02 AM Michael Paquier <[email protected]> wrote:
>
> On Thu, Dec 19, 2024 at 02:28:21PM +0800, jian he wrote:
> > Since materialized views have physical storage,
> >
> > we can make materialized views also using COPY table_name, instead of
> > COPY(query).
> >
> > Some simple tests show around %3.7 or 4.3% speed up.
>
> This restriction comes from 3bf3ab8c5636 as such relations may not be
> scannable when they have no data, no? Perhaps this restriction could
> be lifted, but I'd suggest to dig more into the lists, there should be
> arguments and ideas explaining what could be done in this case
> (spoiler: I did not look at that).
> --
Thanks for the suggestion.
it was mentioned in link [1] and [2].
[1] https://www.postgresql.org/message-id/flat/8967.1353167301%40sss.pgh.pa.us#f5e947cfa9357dba780d238f1...
[2] https://www.postgresql.org/message-id/20121116162558.90150%40gmx.com
Basically we want to have the two directions of COPY.
so
copy the_materialized_view to stdout;
copy the_materialized_view from stdin;
both will work fine.
obviously "copy the_materialized_view from stdin; " will not work.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-01-06 09:34 jian he <[email protected]>
parent: jian he <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: jian he @ 2025-01-06 09:34 UTC (permalink / raw)
To: Michael Paquier <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
hi.
about this issue,
last email in 2012 (https://postgr.es/m/[email protected])
"""
Even if it happens to be trivial in the current patch, it's an added
functional requirement that we might later regret having cavalierly
signed up for. And, as noted upthread, relations that support only
one direction of COPY don't exist at the moment; that would be adding
an asymmetry that we might later regret, too.
regards, tom lane
"""
but now we have numerous COPY options that work solely in a single
direction of COPY.
I think now we can make some kind of relation (pg_class.relkind) that
only works in one direction of COPY.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-01-28 02:48 jian he <[email protected]>
parent: jian he <[email protected]>
0 siblings, 2 replies; 28+ messages in thread
From: jian he @ 2025-01-28 02:48 UTC (permalink / raw)
To: pgsql-hackers; +Cc: Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Mon, Jan 6, 2025 at 5:34 PM jian he <[email protected]> wrote:
>
> hi.
>
> about this issue,
> last email in 2012 (https://postgr.es/m/[email protected])
> """
> Even if it happens to be trivial in the current patch, it's an added
> functional requirement that we might later regret having cavalierly
> signed up for. And, as noted upthread, relations that support only
> one direction of COPY don't exist at the moment; that would be adding
> an asymmetry that we might later regret, too.
>
> regards, tom lane
> """
>
> but now we have numerous COPY options that work solely in a single
> direction of COPY.
> I think now we can make some kind of relation (pg_class.relkind) that
> only works in one direction of COPY.
hi.
patch attached.
also cc to Tom,
since at that time, you are against the idea of ``COPY matview TO``.
Attachments:
[text/x-patch] v1-0001-COPY-materialized_view-TO.patch (1.6K, ../../CACJufxHnyH4Zv+RtOABn8-1PjxkKch9R58dGhr+hCnV0LmNxSQ@mail.gmail.com/2-v1-0001-COPY-materialized_view-TO.patch)
download | inline diff:
From a618e72ae33b6688e75dbcfd5674b558e17ee269 Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Tue, 28 Jan 2025 10:46:03 +0800
Subject: [PATCH v1 1/1] COPY materialized_view TO
context: https://postgr.es/m/[email protected]
context: https://www.postgresql.org/message-id/flat/20121116162558.90150%40gmx.com
discussion: https://postgr.es/m/CACJufxHVxnyRYy67hiPePNCPwVBMzhTQ6FaL9_Te5On9udG=yg@mail.gmail.com
---
src/backend/commands/copyto.c | 12 +++++++-----
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 99cb23cb34..13506023f8 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -380,11 +380,13 @@ BeginCopyTo(ParseState *pstate,
RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO variant.")));
else if (rel->rd_rel->relkind == RELKIND_MATVIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from materialized view \"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
+ {
+ if (!RelationIsPopulated(rel))
+ ereport(ERROR,
+ errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg("cannot copy from materialized view when the materialized view is not populated"),
+ errhint("Use the REFRESH MATERIALIZED VIEW command populate the materialized view first."));
+ }
else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
--
2.34.1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-13 06:50 newtglobal postgresql_contributors <[email protected]>
parent: jian he <[email protected]>
1 sibling, 0 replies; 28+ messages in thread
From: newtglobal postgresql_contributors @ 2025-03-13 06:50 UTC (permalink / raw)
To: [email protected]; +Cc: Jian He <[email protected]>
The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested
Hi,
Tested the latest patch that allows direct `COPY` operations on Materialized Views, removing the need for `COPY (SELECT ...)`. This enhancement reduces query overhead, improving performance by **4–5%**.
Example:
Previous approach:
COPY (SELECT * FROM staff_summary) TO STDOUT WITH CSV HEADER;
Optimized approach:
COPY staff_summary TO STDOUT WITH CSV HEADER;
Performance tests were conducted using a Materialized View containing around 80,000 records, confirming that the new approach is faster and more efficient for exporting data.
Regards,
Newt Global PostgreSQL Contributors
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-26 19:04 Kirill Reshke <[email protected]>
parent: jian he <[email protected]>
1 sibling, 1 reply; 28+ messages in thread
From: Kirill Reshke @ 2025-03-26 19:04 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Tue, 28 Jan 2025 at 07:49, jian he <[email protected]> wrote:
>
> On Mon, Jan 6, 2025 at 5:34 PM jian he <[email protected]> wrote:
> >
> > hi.
> >
> > about this issue,
> > last email in 2012 (https://postgr.es/m/[email protected])
> > """
> > Even if it happens to be trivial in the current patch, it's an added
> > functional requirement that we might later regret having cavalierly
> > signed up for. And, as noted upthread, relations that support only
> > one direction of COPY don't exist at the moment; that would be adding
> > an asymmetry that we might later regret, too.
> >
> > regards, tom lane
> > """
> >
> > but now we have numerous COPY options that work solely in a single
> > direction of COPY.
> > I think now we can make some kind of relation (pg_class.relkind) that
> > only works in one direction of COPY.
>
> hi.
> patch attached.
> also cc to Tom,
> since at that time, you are against the idea of ``COPY matview TO``.
Hi! With this patch it is possible to COPY matview TO, but not regular
view, which is surprising. Let's fix that?
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 06:46 jian he <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: jian he @ 2025-03-29 06:46 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Thu, Mar 27, 2025 at 3:04 AM Kirill Reshke <[email protected]> wrote:
> >
> > hi.
> > patch attached.
> > also cc to Tom,
> > since at that time, you are against the idea of ``COPY matview TO``.
>
> Hi! With this patch it is possible to COPY matview TO, but not regular
> view, which is surprising. Let's fix that?
create view v1 as select 1;
copy v1 to stdout;
if you specifying table name, not query, then
{
cstate = BeginCopyTo(pstate, rel, query, relid,
stmt->filename, stmt->is_program,
NULL, stmt->attlist, stmt->options);
*processed = DoCopyTo(cstate); /* copy from database to file *
}
will use {table_beginscan, table_scan_getnextslot, table_endscan}
to output the data.
but views don't have storage, table_beginscan mechanism won't work.
so i don't think this is possible for view.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 12:38 Kirill Reshke <[email protected]>
parent: jian he <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Kirill Reshke @ 2025-03-29 12:38 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Sat, 29 Mar 2025 at 09:47, jian he <[email protected]> wrote:
>
> will use {table_beginscan, table_scan_getnextslot, table_endscan}
> to output the data.
> but views don't have storage, table_beginscan mechanism won't work.
>
> so i don't think this is possible for view.
Well... So you are saying that let us have inconsistent features
because of how things are implemented in core... I don't sure I'm
buying that, but whatever, let's hear some other voices from the
community. My argument is that while we are working on it, perhaps we
should revise certain implementation specifics along the way. However,
this is merely my opinion on the matter.
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 14:40 David G. Johnston <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: David G. Johnston @ 2025-03-29 14:40 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Saturday, March 29, 2025, Kirill Reshke <[email protected]> wrote:
> On Sat, 29 Mar 2025 at 09:47, jian he <[email protected]> wrote:
> >
> > will use {table_beginscan, table_scan_getnextslot, table_endscan}
> > to output the data.
> > but views don't have storage, table_beginscan mechanism won't work.
> >
> > so i don't think this is possible for view.
>
> Well... So you are saying that let us have inconsistent features
> because of how things are implemented in core... I don't sure I'm
> buying that, but whatever, let's hear some other voices from the
> community. My argument is that while we are working on it, perhaps we
> should revise certain implementation specifics along the way. However,
> this is merely my opinion on the matter.
>
At present copy {table} to only exists to support pg_dump. It is not
marketed as a general purpose export facility. While copy {relation} from
is a general purpose importer. Do we want to turn copy to into a general
purpose exporter and start accepting patches to export foreign tables, deal
with partitioned tables, views of both kinds? Given we already accept
things in copy from that copy to cannot produce the symmetry argument seems
flawed.
I’m on board with making copy {relation} to a general purpose export
facility and allowing for incremental implementations as people wish to
spend time developing them. Consistency should not prevent progress here.
On the topic of copy {matview} from, why not permit it? In particular,
with dump/restore we could dump the materialized view and restore it, which
seems like a win in terms of time spent restoring. That wouldn’t be this
patch.
David J.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 16:06 Andrew Dunstan <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Dunstan @ 2025-03-29 16:06 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; +Cc: jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025-03-29 Sa 10:40 AM, David G. Johnston wrote:
> On Saturday, March 29, 2025, Kirill Reshke <[email protected]> wrote:
>
> On Sat, 29 Mar 2025 at 09:47, jian he
> <[email protected]> wrote:
> >
> > will use {table_beginscan, table_scan_getnextslot, table_endscan}
> > to output the data.
> > but views don't have storage, table_beginscan mechanism won't work.
> >
> > so i don't think this is possible for view.
>
> Well... So you are saying that let us have inconsistent features
> because of how things are implemented in core... I don't sure I'm
> buying that, but whatever, let's hear some other voices from the
> community. My argument is that while we are working on it, perhaps we
> should revise certain implementation specifics along the way. However,
> this is merely my opinion on the matter.
>
>
> At present copy {table} to only exists to support pg_dump. It is not
> marketed as a general purpose export facility.
*ahem*
What is your evidence for that proposition? If this were true we would
not support CSV mode, which pg_dump does not use. It might have
limitations, but its use goes far beyond just pg_dump, both in theory
and practice.
cheers
andew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 16:17 David G. Johnston <[email protected]>
parent: Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: David G. Johnston @ 2025-03-29 16:17 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Kirill Reshke <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Sat, Mar 29, 2025 at 9:06 AM Andrew Dunstan <[email protected]> wrote:
>
> On 2025-03-29 Sa 10:40 AM, David G. Johnston wrote:
>
> On Saturday, March 29, 2025, Kirill Reshke <[email protected]> wrote:
>
>> On Sat, 29 Mar 2025 at 09:47, jian he <[email protected]>
>> wrote:
>> >
>> > will use {table_beginscan, table_scan_getnextslot, table_endscan}
>> > to output the data.
>> > but views don't have storage, table_beginscan mechanism won't work.
>> >
>> > so i don't think this is possible for view.
>>
>> Well... So you are saying that let us have inconsistent features
>> because of how things are implemented in core... I don't sure I'm
>> buying that, but whatever, let's hear some other voices from the
>> community. My argument is that while we are working on it, perhaps we
>> should revise certain implementation specifics along the way. However,
>> this is merely my opinion on the matter.
>>
>
> At present copy {table} to only exists to support pg_dump. It is not
> marketed as a general purpose export facility.
>
>
> *ahem*
>
>
> What is your evidence for that proposition? If this were true we would not
> support CSV mode, which pg_dump does not use. It might have limitations,
> but its use goes far beyond just pg_dump, both in theory and practice.
>
>
>
>
"copy {subquery} to" is a general-purpose exporter that makes use of those
additional features. Sure, they also work for the narrowed case of "copy
{relation/table} to" but I make my claim on the very fact that {relation}
cannot be stuff like foreign tables or partitioned tables, which pg_dump
has no need to target.
David J.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 18:56 Andrew Dunstan <[email protected]>
parent: David G. Johnston <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Andrew Dunstan @ 2025-03-29 18:56 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Kirill Reshke <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025-03-29 Sa 12:17 PM, David G. Johnston wrote:
> On Sat, Mar 29, 2025 at 9:06 AM Andrew Dunstan <[email protected]>
> wrote:
>
>
> On 2025-03-29 Sa 10:40 AM, David G. Johnston wrote:
>> On Saturday, March 29, 2025, Kirill Reshke
>> <[email protected]> wrote:
>>
>> On Sat, 29 Mar 2025 at 09:47, jian he
>> <[email protected]> wrote:
>> >
>> > will use {table_beginscan, table_scan_getnextslot,
>> table_endscan}
>> > to output the data.
>> > but views don't have storage, table_beginscan mechanism
>> won't work.
>> >
>> > so i don't think this is possible for view.
>>
>> Well... So you are saying that let us have inconsistent features
>> because of how things are implemented in core... I don't sure I'm
>> buying that, but whatever, let's hear some other voices from the
>> community. My argument is that while we are working on it,
>> perhaps we
>> should revise certain implementation specifics along the way.
>> However,
>> this is merely my opinion on the matter.
>>
>>
>> At present copy {table} to only exists to support pg_dump. It
>> is not marketed as a general purpose export facility.
>
>
> *ahem*
>
>
> What is your evidence for that proposition? If this were true we
> would not support CSV mode, which pg_dump does not use. It might
> have limitations, but its use goes far beyond just pg_dump, both
> in theory and practice.
>
>
>
>
> "copy {subquery} to" is a general-purpose exporter that makes use of
> those additional features. Sure, they also work for the narrowed case
> of "copy {relation/table} to" but I make my claim on the very fact
> that {relation} cannot be stuff like foreign tables or partitioned
> tables, which pg_dump has no need to target.
>
>
I don't believe that the premise supports the conclusion.
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 18:58 David G. Johnston <[email protected]>
parent: Andrew Dunstan <[email protected]>
0 siblings, 2 replies; 28+ messages in thread
From: David G. Johnston @ 2025-03-29 18:58 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; +Cc: Kirill Reshke <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Sat, Mar 29, 2025 at 11:56 AM Andrew Dunstan <[email protected]> wrote:
> I don't believe that the premise supports the conclusion.
>
>
> Regardless, I do support this patch and probably any similar ones proposed
in the future. Do you have an opinion on that?
David J.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 19:27 Kirill Reshke <[email protected]>
parent: David G. Johnston <[email protected]>
1 sibling, 1 reply; 28+ messages in thread
From: Kirill Reshke @ 2025-03-29 19:27 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Sat, 29 Mar 2025 at 19:59, David G. Johnston
<[email protected]> wrote:
> Regardless, I do support this patch and probably any similar ones proposed in the future. Do you have an opinion on that?
>
> David J.
>
I do also support what this patch aims to do, how do you like v1?
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 19:49 David G. Johnston <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: David G. Johnston @ 2025-03-29 19:49 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Sat, Mar 29, 2025 at 12:27 PM Kirill Reshke <[email protected]>
wrote:
> On Sat, 29 Mar 2025 at 19:59, David G. Johnston
> <[email protected]> wrote:
> > Regardless, I do support this patch and probably any similar ones
> proposed in the future. Do you have an opinion on that?
> >
>
> I do also support what this patch aims to do, how do you like v1?
>
>
Seems reasonable, but I don't have enough experience with the codebase in
that area to submit a code review or be aware of any unexpected
side-effects or deficiencies.
David J.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-29 20:01 Andrew Dunstan <[email protected]>
parent: David G. Johnston <[email protected]>
1 sibling, 1 reply; 28+ messages in thread
From: Andrew Dunstan @ 2025-03-29 20:01 UTC (permalink / raw)
To: David G. Johnston <[email protected]>; +Cc: Kirill Reshke <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025-03-29 Sa 2:58 PM, David G. Johnston wrote:
> On Sat, Mar 29, 2025 at 11:56 AM Andrew Dunstan <[email protected]>
> wrote:
>
> I don't believe that the premise supports the conclusion.
>
>
> Regardless, I do support this patch and probably any similar ones
> proposed in the future. Do you have an opinion on that?
>
>
In principle I think it would be good to have COPY materialized_view TO ...
cheers
andrew
--
Andrew Dunstan
EDB:https://www.enterprisedb.com
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-03-31 15:26 Fujii Masao <[email protected]>
parent: Andrew Dunstan <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Fujii Masao @ 2025-03-31 15:26 UTC (permalink / raw)
To: Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; +Cc: Kirill Reshke <[email protected]>; jian he <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025/03/30 5:01, Andrew Dunstan wrote:
>
> On 2025-03-29 Sa 2:58 PM, David G. Johnston wrote:
>> On Sat, Mar 29, 2025 at 11:56 AM Andrew Dunstan <[email protected]> wrote:
>>
>> I don't believe that the premise supports the conclusion.
>>
>>
>> Regardless, I do support this patch and probably any similar ones proposed in the future. Do you have an opinion on that?
>>
>>
>
> In principle I think it would be good to have COPY materialized_view TO ...
I haven't found any reasons to object to this patch for now,
so I have no objections to this change.
Regarding the patch, here are some review comments:
+ errmsg("cannot copy from materialized view when the materialized view is not populated"),
How about including the object name for consistency with
other error messages in BeginCopyTo(), like this?
errmsg("cannot copy from unpopulated materialized view \"%s\"",
RelationGetRelationName(rel)),
+ errhint("Use the REFRESH MATERIALIZED VIEW command populate the materialized view first."));
There seems to be a missing "to" just after "command".
Should it be "Use the REFRESH MATERIALIZED VIEW command to
populate the materialized view first."? Or we could simplify
the hint to match what SELECT on an unpopulated materialized
view logs: "Use the REFRESH MATERIALIZED VIEW command.".
The copy.sgml documentation should clarify that COPY TO can
be used with a materialized view only if it is populated.
Wouldn't it be beneficial to add a regression test to check
whether COPY matview TO works as expected?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 03:12 jian he <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 3 replies; 28+ messages in thread
From: jian he @ 2025-04-01 03:12 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Mon, Mar 31, 2025 at 11:27 PM Fujii Masao
<[email protected]> wrote:
>
> Regarding the patch, here are some review comments:
>
> + errmsg("cannot copy from materialized view when the materialized view is not populated"),
>
> How about including the object name for consistency with
> other error messages in BeginCopyTo(), like this?
>
> errmsg("cannot copy from unpopulated materialized view \"%s\"",
> RelationGetRelationName(rel)),
>
>
> + errhint("Use the REFRESH MATERIALIZED VIEW command populate the materialized view first."));
>
> There seems to be a missing "to" just after "command".
> Should it be "Use the REFRESH MATERIALIZED VIEW command to
> populate the materialized view first."? Or we could simplify
> the hint to match what SELECT on an unpopulated materialized
> view logs: "Use the REFRESH MATERIALIZED VIEW command.".
>
based on your suggestion, i changed it to:
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."));
>
> 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"
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."
>
> Wouldn't it be beneficial to add a regression test to check
> whether COPY matview TO works as expected?
sure.
Attachments:
[text/x-patch] v2-0001-COPY-materialized_view-TO.patch (4.2K, ../../CACJufxF3noNs9O6O3fL783h2-Sy1wfJb=0H+WMFmOLuhkfJw8w@mail.gmail.com/2-v2-0001-COPY-materialized_view-TO.patch)
download | inline diff:
From 3e404817827a58721cf8966080492f1254ea06cb Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Tue, 1 Apr 2025 11:11:32 +0800
Subject: [PATCH v2 1/1] COPY materialized_view TO
generally `COPY table TO` is faster than `COPY (query)`.
since populated materialized view have physical storage, so
this can use table_beginscan, table_endscan to scan a table.
context: https://postgr.es/m/[email protected]
context: https://www.postgresql.org/message-id/flat/20121116162558.90150%40gmx.com
discussion: https://postgr.es/m/CACJufxHVxnyRYy67hiPePNCPwVBMzhTQ6FaL9_Te5On9udG=yg@mail.gmail.com
commitfest entry: https://commitfest.postgresql.org/patch/5533/
---
doc/src/sgml/ref/copy.sgml | 4 ++--
src/backend/commands/copyto.c | 13 ++++++++-----
src/test/regress/expected/copy2.out | 11 +++++++++++
src/test/regress/sql/copy2.sql | 8 ++++++++
4 files changed, 29 insertions(+), 7 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index df093da97c5..c3107488c81 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -520,8 +520,8 @@ COPY <replaceable class="parameter">count</replaceable>
<title>Notes</title>
<para>
- <command>COPY TO</command> can be used only with plain
- tables, not views, and does not copy rows from child tables
+ <command>COPY TO</command> can be used with plain
+ tables and materialized views, not regular views, and does not copy rows from child tables
or child partitions. For example, <literal>COPY <replaceable
class="parameter">table</replaceable> TO</literal> copies
the same rows as <literal>SELECT * FROM ONLY <replaceable
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 84a3f3879a8..eeab225a690 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -653,11 +653,14 @@ BeginCopyTo(ParseState *pstate,
RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO variant.")));
else if (rel->rd_rel->relkind == RELKIND_MATVIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from materialized view \"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
+ {
+ 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."));
+ }
else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out
index 64ea33aeae8..f7aa55e1691 100644
--- a/src/test/regress/expected/copy2.out
+++ b/src/test/regress/expected/copy2.out
@@ -929,3 +929,14 @@ truncate copy_default;
-- DEFAULT cannot be used in COPY TO
copy (select 1 as test) TO stdout with (default '\D');
ERROR: COPY DEFAULT cannot be used with COPY TO
+-- COPY TO with materialized view
+CREATE MATERIALIZED VIEW matview1 AS SELECT 1 as id;
+CREATE MATERIALIZED VIEW matview2 AS SELECT 1 as id WITH NO DATA;
+copy matview1(id) TO stdout with (header);
+id
+1
+copy matview2 TO stdout with (header);
+ERROR: cannot copy from unpopulated materialized view "matview2"
+HINT: Use the REFRESH MATERIALIZED VIEW command to populate the materialized view first.
+DROP MATERIALIZED VIEW matview1;
+DROP MATERIALIZED VIEW matview2;
diff --git a/src/test/regress/sql/copy2.sql b/src/test/regress/sql/copy2.sql
index 45273557ce0..eeed7064315 100644
--- a/src/test/regress/sql/copy2.sql
+++ b/src/test/regress/sql/copy2.sql
@@ -707,3 +707,11 @@ truncate copy_default;
-- DEFAULT cannot be used in COPY TO
copy (select 1 as test) TO stdout with (default '\D');
+
+-- COPY TO with materialized view
+CREATE MATERIALIZED VIEW matview1 AS SELECT 1 as id;
+CREATE MATERIALIZED VIEW matview2 AS SELECT 1 as id WITH NO DATA;
+copy matview1(id) TO stdout with (header);
+copy matview2 TO stdout with (header);
+DROP MATERIALIZED VIEW matview1;
+DROP MATERIALIZED VIEW matview2;
--
2.34.1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 03:50 David G. Johnston <[email protected]>
parent: jian he <[email protected]>
2 siblings, 0 replies; 28+ messages in thread
From: David G. Johnston @ 2025-04-01 03:50 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Fujii Masao <[email protected]>; Andrew Dunstan <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Mon, Mar 31, 2025 at 8:13 PM jian he <[email protected]> wrote:
> On Mon, Mar 31, 2025 at 11:27 PM Fujii Masao
>
> >
> > 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"
>
> 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."
>
>
Those don't address the "populated" aspect of the materialized view.
I'm unclear ATM (can check later if needed) if populated means "non-empty"
or simply "had refresh executed at least once on it". i.e., if the refresh
produces zero rows stored in the MV is it still populated? I'm guessing
yes; and this only pertains to "WITH NO DATA", which itself already calls
out that "...and cannot be queried until RMV is used". I find it of
marginal usefulness to bring that distinction over to COPY TO absent people
showing up confused about the error message, which likely will be quite
rare. That said I'd probably settle with:
COPY TO can only be used with plain tables and populated
materialized views. It does not copy rows from child tables
or child partitions (i.e., copy table to copies the same rows as
select * from only table). The syntax COPY (select * from table) TO ...
can be used to dump all of the rows in an inheritance hierarchy,
partitioned table, or foreign table; as well as ordinary view results.
Curious about sequences; no way to name an index here.
I'm second-guessing why "composite type" shows up in the glossary under
"Relation"...though that originally came up IIRC discussing namespaces.
David J.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 09:45 vignesh C <[email protected]>
parent: jian he <[email protected]>
2 siblings, 1 reply; 28+ messages in thread
From: vignesh C @ 2025-04-01 09:45 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Fujii Masao <[email protected]>; Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Tue, 1 Apr 2025 at 08:43, jian he <[email protected]> wrote:
>
> On Mon, Mar 31, 2025 at 11:27 PM Fujii Masao
> <[email protected]> wrote:
> >
> > Regarding the patch, here are some review comments:
> >
> > + errmsg("cannot copy from materialized view when the materialized view is not populated"),
> >
> > How about including the object name for consistency with
> > other error messages in BeginCopyTo(), like this?
> >
> > errmsg("cannot copy from unpopulated materialized view \"%s\"",
> > RelationGetRelationName(rel)),
> >
> >
> > + errhint("Use the REFRESH MATERIALIZED VIEW command populate the materialized view first."));
> >
> > There seems to be a missing "to" just after "command".
> > Should it be "Use the REFRESH MATERIALIZED VIEW command to
> > populate the materialized view first."? Or we could simplify
> > the hint to match what SELECT on an unpopulated materialized
> > view logs: "Use the REFRESH MATERIALIZED VIEW command.".
> >
> based on your suggestion, i changed it to:
>
> 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."));
>
>
> >
> > 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"
>
> 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."
One thing I noticed was that if the materialized view is not refreshed
user will get stale data:
postgres=# create table t1(c1 int);
CREATE TABLE
postgres=# create materialized view mv2 as select * from t1;
SELECT 0
postgres=# insert into t1 values(10);
INSERT 0 1
postgres=# select * from t1;
c1
----
10
(1 row)
-- Before refresh the data will not be selected
postgres=# copy mv2 to stdout with (header);
c1
-- After refresh the data will be available
postgres=# refresh materialized view mv2;
REFRESH MATERIALIZED VIEW
postgres=# copy mv2 to stdout with (header);
c1
10
Should we document this?
The following can be changed to keep it consistent:
+copy matview1(id) TO stdout with (header);
+copy matview2 TO stdout with (header);
To:
COPY matview1(id) TO stdout with (header);
COPY matview2 TO stdout with (header);
Regards,
Vignesh
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 10:18 Kirill Reshke <[email protected]>
parent: vignesh C <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Kirill Reshke @ 2025-04-01 10:18 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: jian he <[email protected]>; Fujii Masao <[email protected]>; Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Tue, 1 Apr 2025, 11:45 vignesh C, <[email protected]> wrote:
>
> One thing I noticed was that if the materialized view is not refreshed
> user will get stale data
>
> Should we document this?
>
Does this patch alter thus behaviour? User will get stale data even on
HEAD, why should we take a care within this thread?
>
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 13:52 vignesh C <[email protected]>
parent: Kirill Reshke <[email protected]>
0 siblings, 2 replies; 28+ messages in thread
From: vignesh C @ 2025-04-01 13:52 UTC (permalink / raw)
To: Kirill Reshke <[email protected]>; +Cc: jian he <[email protected]>; Fujii Masao <[email protected]>; Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Tue, 1 Apr 2025 at 15:49, Kirill Reshke <[email protected]> wrote:
>
> On Tue, 1 Apr 2025, 11:45 vignesh C, <[email protected]> wrote:
>>
>>
>> One thing I noticed was that if the materialized view is not refreshed
>> user will get stale data
>>
>> Should we document this?
>
> Does this patch alter thus behaviour? User will get stale data even on HEAD, why should we take a care within this thread?
We are not changing the existing behavior. However, since copying data
from large tables can take a significant amount of time, would it be
helpful to add a cautionary note advising users to refresh the
materialized view before running copy command to avoid stale data?
This could prevent users from realizing the issue only after running
the copy operation, which would then require them to run it again. If
you think this is already obvious, then the note may not be necessary.
Regards,
Vignesh
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 14:14 Kirill Reshke <[email protected]>
parent: vignesh C <[email protected]>
1 sibling, 0 replies; 28+ messages in thread
From: Kirill Reshke @ 2025-04-01 14:14 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: jian he <[email protected]>; Fujii Masao <[email protected]>; Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Tue, 1 Apr 2025 at 15:52, vignesh C <[email protected]> wrote:
>
> On Tue, 1 Apr 2025 at 15:49, Kirill Reshke <[email protected]> wrote:
> >
> > On Tue, 1 Apr 2025, 11:45 vignesh C, <[email protected]> wrote:
> >>
> >>
> >> One thing I noticed was that if the materialized view is not refreshed
> >> user will get stale data
> >>
> >> Should we document this?
> >
> > Does this patch alter thus behaviour? User will get stale data even on HEAD, why should we take a care within this thread?
>
> We are not changing the existing behavior. However, since copying data
> from large tables can take a significant amount of time, would it be
> helpful to add a cautionary note advising users to refresh the
> materialized view before running copy command to avoid stale data?
> This could prevent users from realizing the issue only after running
> the copy operation, which would then require them to run it again.
Yes, agree, +1 on that.
> If
> you think this is already obvious, then the note may not be necessary.
I don't think this is already obvious, but my objection is that we
should maybe discuss this as a separate issue (in a separate patch).
Looks like fixing this together with code commit is too much at once.
I prefer a one-commit-for-one-purpose style.
> Regards,
> Vignesh
--
Best regards,
Kirill Reshke
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-01 14:27 David G. Johnston <[email protected]>
parent: vignesh C <[email protected]>
1 sibling, 0 replies; 28+ messages in thread
From: David G. Johnston @ 2025-04-01 14:27 UTC (permalink / raw)
To: vignesh C <[email protected]>; +Cc: Kirill Reshke <[email protected]>; jian he <[email protected]>; Fujii Masao <[email protected]>; Andrew Dunstan <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Tue, Apr 1, 2025 at 6:52 AM vignesh C <[email protected]> wrote:
>
> We are not changing the existing behavior. However, since copying data
> from large tables can take a significant amount of time, would it be
> helpful to add a cautionary note advising users to refresh the
> materialized view before running copy command to avoid stale data?
>
>
No, for the same reason the caveat about WITH NO DATA need not be mentioned
either. These are the inherent properties/trade-offs of using a
materialized view versus a normal view. They are discussed when talking
about the creation and usage of materialized views specifically. Features
that interact with materialized views consistent with these two documented
properties do not need to point out that fact. i.e., the existing
non-documenting of those two points in SELECT is appropriate, and should be
emulated in COPY. The output doesn't change if you write "copy table"
instead of "copy (select * from table)" so nothing needs to be pointed out.
David J.
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-02 15:19 Fujii Masao <[email protected]>
parent: jian he <[email protected]>
2 siblings, 1 reply; 28+ messages in thread
From: Fujii Masao @ 2025-04-02 15:19 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025/04/01 12:12, jian he wrote:
> On Mon, Mar 31, 2025 at 11:27 PM Fujii Masao
> <[email protected]> wrote:
>>
>> Regarding the patch, here are some review comments:
>>
>> + errmsg("cannot copy from materialized view when the materialized view is not populated"),
>>
>> How about including the object name for consistency with
>> other error messages in BeginCopyTo(), like this?
>>
>> errmsg("cannot copy from unpopulated materialized view \"%s\"",
>> RelationGetRelationName(rel)),
>>
>>
>> + errhint("Use the REFRESH MATERIALIZED VIEW command populate the materialized view first."));
>>
>> There seems to be a missing "to" just after "command".
>> Should it be "Use the REFRESH MATERIALIZED VIEW command to
>> populate the materialized view first."? Or we could simplify
>> the hint to match what SELECT on an unpopulated materialized
>> view logs: "Use the REFRESH MATERIALIZED VIEW command.".
>>
> based on your suggestion, i changed it to:
Thanks for updating the patch!
>
> 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.
>> 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 same rows 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."
-------------
>> Wouldn't it be beneficial to add a regression test to check
>> whether COPY matview TO works as expected?
> sure.
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?
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-03 08:53 jian he <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: jian he @ 2025-04-03 08:53 UTC (permalink / raw)
To: Fujii Masao <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On Wed, Apr 2, 2025 at 11:20 PM Fujii Masao <[email protected]> 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 same rows 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.
Attachments:
[text/x-patch] v3-0001-COPY-materialized_view-TO.patch (4.6K, ../../CACJufxEA-_hUmg=eMrEpd-BK56qXyu9a1izsRHH_8Amw3MXP2Q@mail.gmail.com/2-v3-0001-COPY-materialized_view-TO.patch)
download | inline diff:
From ba91ac0d9403f304d6c72683ca2c471e2c3c3ecc Mon Sep 17 00:00:00 2001
From: jian he <[email protected]>
Date: Thu, 3 Apr 2025 16:45:48 +0800
Subject: [PATCH v3 1/1] COPY materialized_view TO
generally `COPY table TO` is faster than `COPY (query)`.
since populated materialized view have physical storage, so
this can use table_beginscan, table_endscan to scan a table.
context: https://postgr.es/m/[email protected]
context: https://www.postgresql.org/message-id/flat/20121116162558.90150%40gmx.com
discussion: https://postgr.es/m/CACJufxHVxnyRYy67hiPePNCPwVBMzhTQ6FaL9_Te5On9udG=yg@mail.gmail.com
commitfest entry: https://commitfest.postgresql.org/patch/5533/
---
doc/src/sgml/ref/copy.sgml | 14 ++++++++------
src/backend/commands/copyto.c | 13 ++++++++-----
src/test/regress/expected/copy.out | 11 +++++++++++
src/test/regress/sql/copy.sql | 8 ++++++++
4 files changed, 35 insertions(+), 11 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index df093da97c5..c855e98f757 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -520,12 +520,14 @@ COPY <replaceable class="parameter">count</replaceable>
<title>Notes</title>
<para>
- <command>COPY TO</command> can be used only with plain
- tables, not views, and does not copy rows from child tables
- or child partitions. 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>.
+ <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 syntax <literal>COPY (SELECT * FROM <replaceable
class="parameter">table</replaceable>) TO ...</literal> can be used to
dump all of the rows in an inheritance hierarchy, partitioned table,
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 84a3f3879a8..9233cbcecb4 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -653,11 +653,14 @@ BeginCopyTo(ParseState *pstate,
RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO variant.")));
else if (rel->rd_rel->relkind == RELKIND_MATVIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from materialized view \"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
+ {
+ 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."));
+ }
else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index 06bae8c61ae..39af90f5292 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -338,3 +338,14 @@ create foreign table copytest_foreign_table (a int) server copytest_server;
copy copytest_foreign_table from stdin (freeze);
ERROR: cannot perform COPY FREEZE on a foreign table
rollback;
+-- COPY TO with materialized view
+CREATE MATERIALIZED VIEW matview1 AS SELECT 1 as id;
+CREATE MATERIALIZED VIEW matview2 AS SELECT 1 as id WITH NO DATA;
+COPY matview1(id) TO stdout WITH (header);
+id
+1
+COPY matview2 TO stdout WITH (header);
+ERROR: cannot copy from unpopulated materialized view "matview2"
+HINT: Use the REFRESH MATERIALIZED VIEW command.
+DROP MATERIALIZED VIEW matview1;
+DROP MATERIALIZED VIEW matview2;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index 3009bdfdf89..5df65924f8b 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -366,3 +366,11 @@ copy copytest_foreign_table from stdin (freeze);
1
\.
rollback;
+
+-- COPY TO with materialized view
+CREATE MATERIALIZED VIEW matview1 AS SELECT 1 as id;
+CREATE MATERIALIZED VIEW matview2 AS SELECT 1 as id WITH NO DATA;
+COPY matview1(id) TO stdout WITH (header);
+COPY matview2 TO stdout WITH (header);
+DROP MATERIALIZED VIEW matview1;
+DROP MATERIALIZED VIEW matview2;
--
2.34.1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-03 11:46 Fujii Masao <[email protected]>
parent: jian he <[email protected]>
0 siblings, 1 reply; 28+ messages in thread
From: Fujii Masao @ 2025-04-03 11:46 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025/04/03 17:53, jian he wrote:
> On Wed, Apr 2, 2025 at 11:20 PM Fujii Masao <[email protected]> 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 same rows 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
From 18bdeab3d74e539835d59c9a86da8f4ccf2243be Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Thu, 3 Apr 2025 18:41:44 +0900
Subject: [PATCH v4] Allow "COPY table TO" command to copy rows from
materialized views.
Previously, "COPY table TO" command worked only with plain tables and
did not support materialized views, even when they were populated and
had physical storage. To copy rows from materialized views,
"COPY (query) TO" command had to be used, instead.
This commit extends "COPY table TO" to support populated materialized
views directly, improving usability and performance, as "COPY table TO"
is generally faster than "COPY (query) TO". Note that copying from
unpopulated materialized views will still result in an error.
Author: jian he <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Reviewed-by: David G. Johnston <[email protected]>
Reviewed-by: Vignesh C <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/CACJufxHVxnyRYy67hiPePNCPwVBMzhTQ6FaL9_Te5On9udG=yg@mail.gmail.com
---
doc/src/sgml/ref/copy.sgml | 20 ++++++++++----------
src/backend/commands/copyto.c | 13 ++++++++-----
src/test/regress/expected/copy.out | 12 ++++++++++++
src/test/regress/sql/copy.sql | 9 +++++++++
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index df093da97c5..d6859276bed 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -520,16 +520,16 @@ COPY <replaceable class="parameter">count</replaceable>
<title>Notes</title>
<para>
- <command>COPY TO</command> can be used only with plain
- tables, not views, and does not copy rows from child tables
- or child partitions. 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>.
- The syntax <literal>COPY (SELECT * FROM <replaceable
- class="parameter">table</replaceable>) TO ...</literal> can be used to
- dump all of the rows in an inheritance hierarchy, partitioned table,
- or view.
+ <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.
+ To copy all rows from such relations, use <literal>COPY (SELECT * FROM
+ <replaceable class="parameter">table</replaceable>) TO</literal>.
</para>
<para>
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 84a3f3879a8..f87e405351d 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -653,11 +653,14 @@ BeginCopyTo(ParseState *pstate,
RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO variant.")));
else if (rel->rd_rel->relkind == RELKIND_MATVIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from materialized view \"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
+ {
+ 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."));
+ }
else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index 06bae8c61ae..8d5a06563c4 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -338,3 +338,15 @@ create foreign table copytest_foreign_table (a int) server copytest_server;
copy copytest_foreign_table from stdin (freeze);
ERROR: cannot perform COPY FREEZE on a foreign table
rollback;
+-- Tests for COPY TO with materialized views.
+-- COPY TO should fail for an unpopulated materialized view
+-- but succeed for a populated one.
+CREATE MATERIALIZED VIEW copytest_mv AS SELECT 1 AS id WITH NO DATA;
+COPY copytest_mv(id) TO stdout WITH (header);
+ERROR: cannot copy from unpopulated materialized view "copytest_mv"
+HINT: Use the REFRESH MATERIALIZED VIEW command.
+REFRESH MATERIALIZED VIEW copytest_mv;
+COPY copytest_mv(id) TO stdout WITH (header);
+id
+1
+DROP MATERIALIZED VIEW copytest_mv;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index 3009bdfdf89..f0b88a23db8 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -366,3 +366,12 @@ copy copytest_foreign_table from stdin (freeze);
1
\.
rollback;
+
+-- Tests for COPY TO with materialized views.
+-- COPY TO should fail for an unpopulated materialized view
+-- but succeed for a populated one.
+CREATE MATERIALIZED VIEW copytest_mv AS SELECT 1 AS id WITH NO DATA;
+COPY copytest_mv(id) TO stdout WITH (header);
+REFRESH MATERIALIZED VIEW copytest_mv;
+COPY copytest_mv(id) TO stdout WITH (header);
+DROP MATERIALIZED VIEW copytest_mv;
--
2.48.1
Attachments:
[text/plain] v4-0001-Allow-COPY-table-TO-command-to-copy-rows-from-mat.patch (5.4K, ../../[email protected]/2-v4-0001-Allow-COPY-table-TO-command-to-copy-rows-from-mat.patch)
download | inline diff:
From 18bdeab3d74e539835d59c9a86da8f4ccf2243be Mon Sep 17 00:00:00 2001
From: Fujii Masao <[email protected]>
Date: Thu, 3 Apr 2025 18:41:44 +0900
Subject: [PATCH v4] Allow "COPY table TO" command to copy rows from
materialized views.
Previously, "COPY table TO" command worked only with plain tables and
did not support materialized views, even when they were populated and
had physical storage. To copy rows from materialized views,
"COPY (query) TO" command had to be used, instead.
This commit extends "COPY table TO" to support populated materialized
views directly, improving usability and performance, as "COPY table TO"
is generally faster than "COPY (query) TO". Note that copying from
unpopulated materialized views will still result in an error.
Author: jian he <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Reviewed-by: David G. Johnston <[email protected]>
Reviewed-by: Vignesh C <[email protected]>
Reviewed-by: Fujii Masao <[email protected]>
Discussion: https://postgr.es/m/CACJufxHVxnyRYy67hiPePNCPwVBMzhTQ6FaL9_Te5On9udG=yg@mail.gmail.com
---
doc/src/sgml/ref/copy.sgml | 20 ++++++++++----------
src/backend/commands/copyto.c | 13 ++++++++-----
src/test/regress/expected/copy.out | 12 ++++++++++++
src/test/regress/sql/copy.sql | 9 +++++++++
4 files changed, 39 insertions(+), 15 deletions(-)
diff --git a/doc/src/sgml/ref/copy.sgml b/doc/src/sgml/ref/copy.sgml
index df093da97c5..d6859276bed 100644
--- a/doc/src/sgml/ref/copy.sgml
+++ b/doc/src/sgml/ref/copy.sgml
@@ -520,16 +520,16 @@ COPY <replaceable class="parameter">count</replaceable>
<title>Notes</title>
<para>
- <command>COPY TO</command> can be used only with plain
- tables, not views, and does not copy rows from child tables
- or child partitions. 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>.
- The syntax <literal>COPY (SELECT * FROM <replaceable
- class="parameter">table</replaceable>) TO ...</literal> can be used to
- dump all of the rows in an inheritance hierarchy, partitioned table,
- or view.
+ <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.
+ To copy all rows from such relations, use <literal>COPY (SELECT * FROM
+ <replaceable class="parameter">table</replaceable>) TO</literal>.
</para>
<para>
diff --git a/src/backend/commands/copyto.c b/src/backend/commands/copyto.c
index 84a3f3879a8..f87e405351d 100644
--- a/src/backend/commands/copyto.c
+++ b/src/backend/commands/copyto.c
@@ -653,11 +653,14 @@ BeginCopyTo(ParseState *pstate,
RelationGetRelationName(rel)),
errhint("Try the COPY (SELECT ...) TO variant.")));
else if (rel->rd_rel->relkind == RELKIND_MATVIEW)
- ereport(ERROR,
- (errcode(ERRCODE_WRONG_OBJECT_TYPE),
- errmsg("cannot copy from materialized view \"%s\"",
- RelationGetRelationName(rel)),
- errhint("Try the COPY (SELECT ...) TO variant.")));
+ {
+ 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."));
+ }
else if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
ereport(ERROR,
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
diff --git a/src/test/regress/expected/copy.out b/src/test/regress/expected/copy.out
index 06bae8c61ae..8d5a06563c4 100644
--- a/src/test/regress/expected/copy.out
+++ b/src/test/regress/expected/copy.out
@@ -338,3 +338,15 @@ create foreign table copytest_foreign_table (a int) server copytest_server;
copy copytest_foreign_table from stdin (freeze);
ERROR: cannot perform COPY FREEZE on a foreign table
rollback;
+-- Tests for COPY TO with materialized views.
+-- COPY TO should fail for an unpopulated materialized view
+-- but succeed for a populated one.
+CREATE MATERIALIZED VIEW copytest_mv AS SELECT 1 AS id WITH NO DATA;
+COPY copytest_mv(id) TO stdout WITH (header);
+ERROR: cannot copy from unpopulated materialized view "copytest_mv"
+HINT: Use the REFRESH MATERIALIZED VIEW command.
+REFRESH MATERIALIZED VIEW copytest_mv;
+COPY copytest_mv(id) TO stdout WITH (header);
+id
+1
+DROP MATERIALIZED VIEW copytest_mv;
diff --git a/src/test/regress/sql/copy.sql b/src/test/regress/sql/copy.sql
index 3009bdfdf89..f0b88a23db8 100644
--- a/src/test/regress/sql/copy.sql
+++ b/src/test/regress/sql/copy.sql
@@ -366,3 +366,12 @@ copy copytest_foreign_table from stdin (freeze);
1
\.
rollback;
+
+-- Tests for COPY TO with materialized views.
+-- COPY TO should fail for an unpopulated materialized view
+-- but succeed for a populated one.
+CREATE MATERIALIZED VIEW copytest_mv AS SELECT 1 AS id WITH NO DATA;
+COPY copytest_mv(id) TO stdout WITH (header);
+REFRESH MATERIALIZED VIEW copytest_mv;
+COPY copytest_mv(id) TO stdout WITH (header);
+DROP MATERIALIZED VIEW copytest_mv;
--
2.48.1
^ permalink raw reply [nested|flat] 28+ messages in thread
* Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query).
@ 2025-04-04 13:39 Fujii Masao <[email protected]>
parent: Fujii Masao <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Fujii Masao @ 2025-04-04 13:39 UTC (permalink / raw)
To: jian he <[email protected]>; +Cc: Andrew Dunstan <[email protected]>; David G. Johnston <[email protected]>; Kirill Reshke <[email protected]>; pgsql-hackers; Tom Lane <[email protected]>; Michael Paquier <[email protected]>
On 2025/04/03 20:46, Fujii Masao wrote:
> 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.
I've pushed the patch. Thanks!
As a follow-up, it might be worth enhancing psql to also
complete materialized view names after "COPY".
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
^ permalink raw reply [nested|flat] 28+ messages in thread
* [PATCH 1/3] stress test for repack concurrently
@ 2025-12-13 17:13 Mikhail Nikalayeu <[email protected]>
0 siblings, 0 replies; 28+ messages in thread
From: Mikhail Nikalayeu @ 2025-12-13 17:13 UTC (permalink / raw)
---
contrib/amcheck/meson.build | 1 +
contrib/amcheck/t/007_repack_1.pl | 121 ++++++++++++++++++++++++++++++
doc/src/sgml/regress.sgml | 16 ++++
3 files changed, 138 insertions(+)
create mode 100644 contrib/amcheck/t/007_repack_1.pl
diff --git a/contrib/amcheck/meson.build b/contrib/amcheck/meson.build
index d5137ef691d..cb4bc32e98a 100644
--- a/contrib/amcheck/meson.build
+++ b/contrib/amcheck/meson.build
@@ -50,6 +50,7 @@ tests += {
't/004_verify_nbtree_unique.pl',
't/005_pitr.pl',
't/006_verify_gin.pl',
+ 't/007_repack_1.pl',
],
},
}
diff --git a/contrib/amcheck/t/007_repack_1.pl b/contrib/amcheck/t/007_repack_1.pl
new file mode 100644
index 00000000000..17f3caa3a38
--- /dev/null
+++ b/contrib/amcheck/t/007_repack_1.pl
@@ -0,0 +1,121 @@
+# Copyright (c) 2021-2026, PostgreSQL Global Development Group
+
+# Test REPACK CONCURRENTLY with concurrent modifications
+use strict;
+use warnings FATAL => 'all';
+
+use PostgreSQL::Test::Cluster;
+use PostgreSQL::Test::Utils;
+
+use Test::More;
+
+# PG_TEST_EXTRA carries a numerical stress value; 0 disables the test,
+# 1 means the test takes about 6 seconds, and the increase should be
+# roughly linear.
+my $matched = ($ENV{PG_TEST_EXTRA} // '') =~ /\bstress_concurrently(?:=(?<stressval>[0-9]*))?\b/;
+my $stressval = $+{stressval} // 1;
+if (!$matched or $stressval == 0)
+{
+ plan skip_all => 'skipping disabled REPACK CONCURRENTLY stress test';
+}
+
+note "stressval is $stressval";
+
+my $node;
+
+# Test set-up
+$node = PostgreSQL::Test::Cluster->new('CIC_test');
+$node->init;
+$node->append_conf('postgresql.conf',
+ 'lock_timeout = ' . (1000 * $PostgreSQL::Test::Utils::timeout_default));
+$node->append_conf(
+ 'postgresql.conf', qq(
+wal_level = logical
+));
+
+my $nrows = 10_000 * $stressval;
+my $duration = 6 * $stressval;
+my $no_hot = int(rand(2));
+
+$node->start;
+$node->safe_psql('postgres', q(CREATE TABLE tbl(id int PRIMARY KEY, val int)));
+
+if ($no_hot)
+{
+ $node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(val);));
+}
+else
+{
+ $node->safe_psql('postgres', q(CREATE INDEX test_idx ON tbl(id);));
+}
+
+
+# Load amcheck
+$node->safe_psql('postgres', q(CREATE EXTENSION amcheck));
+
+# Insert $nrows rows into tbl
+$node->safe_psql('postgres', qq(
+ INSERT INTO tbl SELECT g, g FROM generate_series(1, $nrows) g
+));
+
+my $sum = $node->safe_psql('postgres', q(
+ SELECT SUM(val) AS sum FROM tbl
+));
+
+
+$node->pgbench(
+"--no-vacuum --client=30 --jobs=4 --exit-on-abort -T $duration",
+0,
+[qr{actually processed}],
+[qr{^$}],
+'concurrent operations with REINDEX/CREATE INDEX CONCURRENTLY',
+{
+ 'concurrent_ops' => qq(
+ SELECT pg_try_advisory_lock(42)::integer AS gotlock \\gset
+ \\if :gotlock
+ REPACK (CONCURRENTLY) tbl USING INDEX tbl_pkey;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ REPACK (CONCURRENTLY) tbl USING INDEX test_idx;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ REPACK (CONCURRENTLY) tbl;
+ SELECT bt_index_parent_check('tbl_pkey', heapallindexed => true);
+ SELECT bt_index_parent_check('test_idx', heapallindexed => true);
+ \\sleep 10 ms
+
+ SELECT pg_advisory_unlock(42);
+ \\else
+ \\set num_a random(1, $nrows)
+ \\set num_b random(1, $nrows)
+ \\set diff random(1, 10000)
+ BEGIN;
+ UPDATE tbl SET val = val + :diff WHERE id = :num_a;
+ \\sleep 1 ms
+ UPDATE tbl SET val = val - :diff WHERE id = :num_b;
+ \\sleep 1 ms
+ COMMIT;
+
+ BEGIN
+ --TRANSACTION ISOLATION LEVEL REPEATABLE READ
+ ;
+ SELECT 1;
+ \\sleep 1 ms
+ SELECT COALESCE(SUM(val), 0) AS sum FROM tbl \\gset p_
+ \\if :p_sum != $sum
+ COMMIT;
+ SELECT (:p_sum) / 0;
+ \\endif
+
+ COMMIT;
+ \\endif
+ )
+});
+
+$node->stop;
+
+done_testing();
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index c74941bfbf2..c5b1b79a60c 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -386,6 +386,22 @@ make check-world PG_TEST_EXTRA='kerberos ldap ssl load_balance libpq_encryption'
</listitem>
</varlistentry>
+ <varlistentry>
+ <term><literal>stress_concurrently</literal><optional>=VALUE</optional></term>
+ <listitem>
+ <para>
+ Run some additional, moderately expensive tests for commands with a
+ <literal>CONCURRENTLY</literal> option. Optionally, an integer can be
+ given after an equal sign, which affects how long each such test
+ runs for.
+ Each test is calibrated so that, when given a value of 10,
+ it runs for approximately 60 seconds.
+ If no value is given, 1 is assumed.
+ Explicitly giving a value of 0 causes the test to be skipped.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry>
<term><literal>wal_consistency_checking</literal></term>
<listitem>
--
2.47.3
--tbdehtstrqwksfdh
Content-Type: text/x-diff; charset=utf-8
Content-Disposition: attachment;
filename="0002-one-more-stress-test-for-repack-concurrently.patch"
^ permalink raw reply [nested|flat] 28+ messages in thread
end of thread, other threads:[~2025-12-13 17:13 UTC | newest]
Thread overview: 28+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-12-20 02:53 Re: in BeginCopyTo make materialized view using COPY TO instead of COPY (query). jian he <[email protected]>
2025-01-06 09:34 ` jian he <[email protected]>
2025-01-28 02:48 ` jian he <[email protected]>
2025-03-13 06:50 ` newtglobal postgresql_contributors <[email protected]>
2025-03-26 19:04 ` Kirill Reshke <[email protected]>
2025-03-29 06:46 ` jian he <[email protected]>
2025-03-29 12:38 ` Kirill Reshke <[email protected]>
2025-03-29 14:40 ` David G. Johnston <[email protected]>
2025-03-29 16:06 ` Andrew Dunstan <[email protected]>
2025-03-29 16:17 ` David G. Johnston <[email protected]>
2025-03-29 18:56 ` Andrew Dunstan <[email protected]>
2025-03-29 18:58 ` David G. Johnston <[email protected]>
2025-03-29 19:27 ` Kirill Reshke <[email protected]>
2025-03-29 19:49 ` David G. Johnston <[email protected]>
2025-03-29 20:01 ` Andrew Dunstan <[email protected]>
2025-03-31 15:26 ` Fujii Masao <[email protected]>
2025-04-01 03:12 ` jian he <[email protected]>
2025-04-01 03:50 ` David G. Johnston <[email protected]>
2025-04-01 09:45 ` vignesh C <[email protected]>
2025-04-01 10:18 ` Kirill Reshke <[email protected]>
2025-04-01 13:52 ` vignesh C <[email protected]>
2025-04-01 14:14 ` Kirill Reshke <[email protected]>
2025-04-01 14:27 ` David G. Johnston <[email protected]>
2025-04-02 15:19 ` Fujii Masao <[email protected]>
2025-04-03 08:53 ` jian he <[email protected]>
2025-04-03 11:46 ` Fujii Masao <[email protected]>
2025-04-04 13:39 ` Fujii Masao <[email protected]>
2025-12-13 17:13 [PATCH 1/3] stress test for repack concurrently Mikhail Nikalayeu <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox