Thread: COPY view

COPY view

From
Karel Zak
Date:
 Hi,

attached is a patch that implements "COPY view TO" feature.

    Karel

Example:

test=# CREATE VIEW vvv AS SELECT a.id, a.data AS d1, b.data AS d2 FROM
tab a, tab2 b WHERE a.id=b.fk;
CREATE VIEW
test=# COPY vvv TO '/tmp/test';
COPY
test=# \! cat /tmp/test
1       aaa     AAA
2       bbb     BBB
3       ccc     CCC
4       ddd     DDD


--
Karel Zak <zakkr@zf.jcu.cz>

Attachment

Re: COPY view

From
Bruce Momjian
Date:
This has been saved for the 8.2 release:

    http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Karel Zak wrote:
>
>  Hi,
>
> attached is a patch that implements "COPY view TO" feature.
>
>     Karel
>
> Example:
>
> test=# CREATE VIEW vvv AS SELECT a.id, a.data AS d1, b.data AS d2 FROM
> tab a, tab2 b WHERE a.id=b.fk;
> CREATE VIEW
> test=# COPY vvv TO '/tmp/test';
> COPY
> test=# \! cat /tmp/test
> 1       aaa     AAA
> 2       bbb     BBB
> 3       ccc     CCC
> 4       ddd     DDD
>
>
> --
> Karel Zak <zakkr@zf.jcu.cz>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: COPY view

From
Bruce Momjian
Date:
This patch implements the COPY VIEW TODO item.  Do we want to apply it
for 8.2?

---------------------------------------------------------------------------

Karel Zak wrote:
>
>  Hi,
>
> attached is a patch that implements "COPY view TO" feature.
>
>     Karel
>
> Example:
>
> test=# CREATE VIEW vvv AS SELECT a.id, a.data AS d1, b.data AS d2 FROM
> tab a, tab2 b WHERE a.id=b.fk;
> CREATE VIEW
> test=# COPY vvv TO '/tmp/test';
> COPY
> test=# \! cat /tmp/test
> 1       aaa     AAA
> 2       bbb     BBB
> 3       ccc     CCC
> 4       ddd     DDD
>
>
> --
> Karel Zak <zakkr@zf.jcu.cz>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Andrew Dunstan
Date:

Bruce Momjian wrote:

>This patch implements the COPY VIEW TODO item.  Do we want to apply it
>for 8.2?
>
>
>

I thought the consensus was that it would be better to do

  COPY (SELECT ...) TO ...

rather than requiring the intermediate creation of a view.

cheers

andrew

Re: COPY view

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
>
> >This patch implements the COPY VIEW TODO item.  Do we want to apply it
> >for 8.2?
> >
> >
> >
>
> I thought the consensus was that it would be better to do
>
>   COPY (SELECT ...) TO ...
>
> rather than requiring the intermediate creation of a view.

Right, but even when we have that, should we be able to dump a view just
like a real table?

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Andrew Dunstan
Date:

Bruce Momjian wrote:

>Andrew Dunstan wrote:
>
>
>>Bruce Momjian wrote:
>>
>>
>>
>>>This patch implements the COPY VIEW TODO item.  Do we want to apply it
>>>for 8.2?
>>>
>>>
>>>
>>>
>>>
>>I thought the consensus was that it would be better to do
>>
>>  COPY (SELECT ...) TO ...
>>
>>rather than requiring the intermediate creation of a view.
>>
>>
>
>Right, but even when we have that, should we be able to dump a view just
>like a real table?
>
>
>

Wouldn't it just be this? :

  COPY (SELECT * from viewname) TO ...

I don't see why views should be special. Tables clearly should be
because we can open them directly.

cheers

andrew

Re: COPY view

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> >Right, but even when we have that, should we be able to dump a view just
> >like a real table?
> >
> >
> >
>
> Wouldn't it just be this? :
>
>   COPY (SELECT * from viewname) TO ...
>
> I don't see why views should be special. Tables clearly should be
> because we can open them directly.

Ah, I didn't think of that.  Good idea.  So we don't need this patch?

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Andrew Dunstan
Date:

Hans-Juergen Schoenig wrote:

> we agreed on the view  solution as people thought that this would be
> better and less intrusive. I was also in favour of the syntax your
> described below but people voted for the view solution which has been
> fully implemented by this patch.
> Btw, it seems to be stable - it has been in production at a customer
> for quite a while now.


See this recent thread:

http://archives.postgresql.org/pgsql-hackers/2006-06/msg00055.php

cheers

andrew



Re: COPY view

From
Bruce Momjian
Date:
Hans-Juergen Schoenig wrote:
> Bruce Momjian wrote:
> >>I don't see why views should be special. Tables clearly should be
> >>because we can open them directly.
> >>
> >>
> >
> >Ah, I didn't think of that.  Good idea.  So we don't need this patch?
> >
> >
> >
>
> why do we agree on a patch, implement it and reject it then?
> would be easier to reject it before actually implementing it ...
> it is quite hard to explain to a customer that something is rejected
> after approval - even if things are written properly ...

Agreed.  The problem with this patch is that originally we just wanted
views, and later the idea of putting a query in there was agreed on, so
the feature request has changed over time.

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Andrew Dunstan
Date:

Hans-Juergen Schoenig wrote:

>
> why do we agree on a patch, implement it and reject it then?
> would be easier to reject it before actually implementing it ...
> it is quite hard to explain to a customer that something is rejected
> after approval - even if things are written properly ...
>
>


That's a good point and I understand the pain.

Could we maybe do  this?: Take the patch as it is now, and if/when we
get the more general syntax we do a little magic under the hood to turn
  COPY viewname TO
into
 COPY (select * from viewname) TO


just a thought

cheers

andrew



Re: COPY view

From
"Jim C. Nasby"
Date:
On Wed, Jun 14, 2006 at 05:19:44PM -0400, Bruce Momjian wrote:
> Hans-Juergen Schoenig wrote:
> > Bruce Momjian wrote:
> > >>I don't see why views should be special. Tables clearly should be
> > >>because we can open them directly.
> > >>
> > >>
> > >
> > >Ah, I didn't think of that.  Good idea.  So we don't need this patch?
> > >
> > >
> > >
> >
> > why do we agree on a patch, implement it and reject it then?
> > would be easier to reject it before actually implementing it ...
> > it is quite hard to explain to a customer that something is rejected
> > after approval - even if things are written properly ...
>
> Agreed.  The problem with this patch is that originally we just wanted
> views, and later the idea of putting a query in there was agreed on, so
> the feature request has changed over time.

BTW, one argument for allowing dumping out of views is that it means
they'd act more like tables; you just COPY viewname TO file.

Also, if copy from select doesn't make it into 8.2, then we should
absolutely put this patch in, so that users at least have something they
can use.
--
Jim C. Nasby, Sr. Engineering Consultant      jnasby@pervasive.com
Pervasive Software      http://pervasive.com    work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf       cell: 512-569-9461

Re: COPY view

From
Bruce Momjian
Date:
Hans-Juergen Schoenig wrote:
>
> >>why do we agree on a patch, implement it and reject it then?
> >>would be easier to reject it before actually implementing it ...
> >>it is quite hard to explain to a customer that something is rejected
> >>after approval - even if things are written properly ...
> >>
> >>
> >
> >Agreed.  The problem with this patch is that originally we just wanted
> >views, and later the idea of putting a query in there was agreed on, so
> >the feature request has changed over time.

When the idea was originally discussed, we didn't want queries because
some thought they would be too much overhead, but later discussion
thought queries would be very useful.  The basic issue is that "ease of
use" is getting more weight than it used to as we expand our user base.

> my original proposal said that we should support SELECT in there.
> before we started to work it was changed to views - now we are moving
> backwards towards the original idea.
> the problem is not that it might have to be changed; the problem is that
> for those people out there who actually put the money on the table this
> way of decision making is not too obvious and will definitely lead to
> future frustration - and this is what we all want to avoid.

We support what we ship, not what others add.

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
>
>
> Hans-Juergen Schoenig wrote:
>
> >
> > why do we agree on a patch, implement it and reject it then?
> > would be easier to reject it before actually implementing it ...
> > it is quite hard to explain to a customer that something is rejected
> > after approval - even if things are written properly ...
> >
> >
>
>
> That's a good point and I understand the pain.
>
> Could we maybe do  this?: Take the patch as it is now, and if/when we
> get the more general syntax we do a little magic under the hood to turn
>   COPY viewname TO
> into
>  COPY (select * from viewname) TO

We could.  But we would do it because we want that behavior on its own,
rather than doing it just to support a feature we added in the past.

The question is, if we were adding the query syntax _now_, would we want
to do views that way?  If so, we can add the patch and just fix it up
when we get the queries.

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Bruce Momjian
Date:
Jim C. Nasby wrote:
> On Wed, Jun 14, 2006 at 05:19:44PM -0400, Bruce Momjian wrote:
> > Hans-Juergen Schoenig wrote:
> > > Bruce Momjian wrote:
> > > >>I don't see why views should be special. Tables clearly should be
> > > >>because we can open them directly.
> > > >>
> > > >>
> > > >
> > > >Ah, I didn't think of that.  Good idea.  So we don't need this patch?
> > > >
> > > >
> > > >
> > >
> > > why do we agree on a patch, implement it and reject it then?
> > > would be easier to reject it before actually implementing it ...
> > > it is quite hard to explain to a customer that something is rejected
> > > after approval - even if things are written properly ...
> >
> > Agreed.  The problem with this patch is that originally we just wanted
> > views, and later the idea of putting a query in there was agreed on, so
> > the feature request has changed over time.
>
> BTW, one argument for allowing dumping out of views is that it means
> they'd act more like tables; you just COPY viewname TO file.

I think the simple argument is that you can SELECT from a table, why not
COPY from it.  Of course copying INTO a view would not work.  :-(

--
  Bruce Momjian   http://candle.pha.pa.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
"Jim C. Nasby"
Date:
On Wed, Jun 14, 2006 at 05:36:25PM -0400, Bruce Momjian wrote:
> Jim C. Nasby wrote:
> > On Wed, Jun 14, 2006 at 05:19:44PM -0400, Bruce Momjian wrote:
> > > Hans-Juergen Schoenig wrote:
> > > > Bruce Momjian wrote:
> > > > >>I don't see why views should be special. Tables clearly should be
> > > > >>because we can open them directly.
> > > > >>
> > > > >>
> > > > >
> > > > >Ah, I didn't think of that.  Good idea.  So we don't need this patch?
> > > > >
> > > > >
> > > > >
> > > >
> > > > why do we agree on a patch, implement it and reject it then?
> > > > would be easier to reject it before actually implementing it ...
> > > > it is quite hard to explain to a customer that something is rejected
> > > > after approval - even if things are written properly ...
> > >
> > > Agreed.  The problem with this patch is that originally we just wanted
> > > views, and later the idea of putting a query in there was agreed on, so
> > > the feature request has changed over time.
> >
> > BTW, one argument for allowing dumping out of views is that it means
> > they'd act more like tables; you just COPY viewname TO file.
>
> I think the simple argument is that you can SELECT from a table, why not
> COPY from it.  Of course copying INTO a view would not work.  :-(

Aside from legacy, if you do COPY tablename, you know that it's going to
be a 'high speed' copy, while presumably COPY (SELECT * FROM tablename)
will have additional overhead. Of course, this is also an argument
against the patch.

Granted, if we wanted to we could put the brains in the code to figure
out if a COPY (SELECT) is actually (SELECT * FROM table), which means we
could use the fast code-path, but I don't think it's worth the effort.
--
Jim C. Nasby, Sr. Engineering Consultant      jnasby@pervasive.com
Pervasive Software      http://pervasive.com    work: 512-231-6117
vcard: http://jim.nasby.net/pervasive.vcf       cell: 512-569-9461

Re: COPY view

From
Bruce Momjian
Date:
Jim C. Nasby wrote:
> On Wed, Jun 14, 2006 at 05:19:44PM -0400, Bruce Momjian wrote:
> > Hans-Juergen Schoenig wrote:
> > > Bruce Momjian wrote:
> > > >>I don't see why views should be special. Tables clearly should be
> > > >>because we can open them directly.
> > > >>
> > > >>
> > > >
> > > >Ah, I didn't think of that.  Good idea.  So we don't need this patch?
> > > >
> > > >
> > > >
> > >
> > > why do we agree on a patch, implement it and reject it then?
> > > would be easier to reject it before actually implementing it ...
> > > it is quite hard to explain to a customer that something is rejected
> > > after approval - even if things are written properly ...
> >
> > Agreed.  The problem with this patch is that originally we just wanted
> > views, and later the idea of putting a query in there was agreed on, so
> > the feature request has changed over time.
>
> BTW, one argument for allowing dumping out of views is that it means
> they'd act more like tables; you just COPY viewname TO file.
>
> Also, if copy from select doesn't make it into 8.2, then we should
> absolutely put this patch in, so that users at least have something they
> can use.

OK, based on this feedback, I am adding COPY VIEW to the patches queue.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Bruce Momjian
Date:
Your patch has been added to the PostgreSQL unapplied patches list at:

    http://momjian.postgresql.org/cgi-bin/pgpatches

It will be applied as soon as one of the PostgreSQL committers reviews
and approves it.

---------------------------------------------------------------------------


Karel Zak wrote:
>
>  Hi,
>
> attached is a patch that implements "COPY view TO" feature.
>
>     Karel
>
> Example:
>
> test=# CREATE VIEW vvv AS SELECT a.id, a.data AS d1, b.data AS d2 FROM
> tab a, tab2 b WHERE a.id=b.fk;
> CREATE VIEW
> test=# COPY vvv TO '/tmp/test';
> COPY
> test=# \! cat /tmp/test
> 1       aaa     AAA
> 2       bbb     BBB
> 3       ccc     CCC
> 4       ddd     DDD
>
>
> --
> Karel Zak <zakkr@zf.jcu.cz>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: COPY view

From
Tom Lane
Date:
Bruce Momjian <bruce@momjian.us> writes:
> OK, based on this feedback, I am adding COPY VIEW to the patches queue.

I think we have other things that demand our attention more than a
half-baked feature.

            regards, tom lane

Re: COPY view

From
Bruce Momjian
Date:
Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > OK, based on this feedback, I am adding COPY VIEW to the patches queue.
>
> I think we have other things that demand our attention more than a
> half-baked feature.

Well, the patch was submitted in time, and it is a desired feature.  If
we want to hold it for 8.3 due to lack of time, we can, but I don't
think we can decide now that it must wait.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] COPY view

From
Stefan Kaltenbrunner
Date:
Bruce Momjian wrote:
> Tom Lane wrote:
>> Bruce Momjian <bruce@momjian.us> writes:
>>> OK, based on this feedback, I am adding COPY VIEW to the patches queue.
>> I think we have other things that demand our attention more than a
>> half-baked feature.
>
> Well, the patch was submitted in time, and it is a desired feature.  If
> we want to hold it for 8.3 due to lack of time, we can, but I don't
> think we can decide now that it must wait.


well I thought the agreed approach to that was allowing COPY from
arbitrary expressions without the need to go through the extra CREATE
VIEW step?


Stefan

Re: [HACKERS] COPY view

From
Tom Lane
Date:
Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
> Bruce Momjian wrote:
>> Well, the patch was submitted in time, and it is a desired feature.  If
>> we want to hold it for 8.3 due to lack of time, we can, but I don't
>> think we can decide now that it must wait.

> well I thought the agreed approach to that was allowing COPY from
> arbitrary expressions without the need to go through the extra CREATE
> VIEW step?

Exactly.  This is not the feature that was agreed to.  Just because we
have a patch for it doesn't mean that we have to put it in.  If we do
put it in, we'll be stuck carrying that feature forever, even after
someone gets around to doing it right.

            regards, tom lane

Re: [HACKERS] COPY view

From
Andrew Dunstan
Date:
Stefan Kaltenbrunner wrote:
> Bruce Momjian wrote:
>
>> Tom Lane wrote:
>>
>>> Bruce Momjian <bruce@momjian.us> writes:
>>>
>>>> OK, based on this feedback, I am adding COPY VIEW to the patches queue.
>>>>
>>> I think we have other things that demand our attention more than a
>>> half-baked feature.
>>>
>> Well, the patch was submitted in time, and it is a desired feature.  If
>> we want to hold it for 8.3 due to lack of time, we can, but I don't
>> think we can decide now that it must wait.
>>
>
>
> well I thought the agreed approach to that was allowing COPY from
> arbitrary expressions without the need to go through the extra CREATE
> VIEW step?
>
>
>

Well, it's been a bit of a mess, unfortunately, and I can understand
people feeling aggrieved.

I think there is general agreement that we want to be able to do:

  COPY (SELECT ... ) TO ...

When we have that it would not be unreasonable to have a special case
for views which would transparently rewrite

  COPY VIEWNAME TO

as

  COPY (SELECT * FROM VIEWNAME) TO

So we would not necessarily be adopting a feature we don't want in the
long run, from a user visibility angle.


The issue seems to be that in adopting the present patch we would be
incorporating some code we will essentially have to abandon  when we get
the feature we all really want, and which we hope will be available for
8.3. On that basis I can certainly appreciate Tom's reluctance to adopt
the patch.

It's a close call. On balance I'd be inclined to accept the patch if it
reviews OK, even though we will throw the code away soon (we hope).

cheers

andrew

Re: [HACKERS] COPY view

From
Tom Lane
Date:
Andrew Dunstan <andrew@dunslane.net> writes:
> It's a close call. On balance I'd be inclined to accept the patch if it
> reviews OK, even though we will throw the code away soon (we hope).

Well, the patch seems pretty ugly code-wise as well.  I'd be willing to
clean it up if I thought it wouldn't ultimately get yanked out again,
but I'm not sure that I see the point if we think it's a dead end.

It doesn't come close to applying to CVS HEAD, either :-(

            regards, tom lane

Re: [HACKERS] COPY view

From
Andrew Dunstan
Date:
Hans-Juergen Schoenig wrote:
> Tom Lane wrote:
>> Stefan Kaltenbrunner <stefan@kaltenbrunner.cc> writes:
>>
>>> Bruce Momjian wrote:
>>>
>>>> Well, the patch was submitted in time, and it is a desired
>>>> feature.  If
>>>> we want to hold it for 8.3 due to lack of time, we can, but I don't
>>>> think we can decide now that it must wait.
>>>>
>>
>>
>>> well I thought the agreed approach to that was allowing COPY from
>>> arbitrary expressions without the need to go through the extra CREATE
>>> VIEW step?
>>>
>>
>> Exactly.  This is not the feature that was agreed to.  Just because we
>> have a patch for it doesn't mean that we have to put it in.  If we do
>> put it in, we'll be stuck carrying that feature forever, even after
>> someone gets around to doing it right.
>>
>>             regards, tom lane
>>
>
>
> It has been made as "COPY FROM / TO view" because people wanted it to
> be done that way.
> My original proposal was in favour of arbitrary SELECTs (just like
> proposed by the TODO list) but this was rejected. So, we did it that
> way (had to explain to customer why views are better). Now everybody
> wants the original select which was proposed.
>
> I can understand if things are not committed because of bad code
> quality or whatever but to be honest: It is more of less frustrating
> if things are done differently because of community wish and then
> rejected because things are not done the original way ...
>
> Things have been submitted months ago and now we are short of time. I
> think everybody on the list is going a superior job but after 6 years
> I still have no idea how patches are treated ;).
>
>

There's nothing hidden (unless it's also hidden from me ;-) )

I take it that when you talk about "we did this" you are referring to
the patch from Karel Zak.

I have had a quick look at that patch, and apart from not applying
cleanly to the current CVS tree (which isn't your fault as the patch has
been sitting around for so long) it is also missing regression tests and
docs. That's without even looking at code quality. So, how quickly can
you fix those 3 things?

cheers

andrew


Re: [HACKERS] COPY view

From
Alvaro Herrera
Date:
Hans-Juergen Schoenig wrote:

> It has been made as "COPY FROM / TO view" because people wanted it to be
> done that way.
> My original proposal was in favour of arbitrary SELECTs (just like
> proposed by the TODO list) but this was rejected. So, we did it that way
> (had to explain to customer why views are better). Now everybody wants
> the original select which was proposed.

This is not the first time this happens.  It has happened to Simon Riggs
at least once and to me as well.  Sometimes "the community" just doesn't
realize what it wants, until what it think it wants is done and then
realizes it wants something else.

It is frustrating, but I don't see how to do things differently.


> Things have been submitted months ago and now we are short of time. I
> think everybody on the list is going a superior job but after 6 years I
> still have no idea how patches are treated ;).

It sucks that patches are posted and no action is taken on them for
months.  I agree with that.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: [HACKERS] COPY view

From
Tom Lane
Date:
Alvaro Herrera <alvherre@commandprompt.com> writes:
> It sucks that patches are posted and no action is taken on them for
> months.  I agree with that.

This particular patch was originally posted during the 8.1 feature
freeze window (2005-09-29), so it was doomed to a certain amount of
languishing on the to-worry-about-later list in any case.  We should
have gotten around to reviewing it sooner than we did (the followup
discussion was around 2006-06-14), but there was still plenty of time
at that point to rework it per the discussion and get it into 8.2.

As I see it, we've effectively got a patch that was rejected once,
and Bruce wants to apply it anyway because no replacement has been
forthcoming.

            regards, tom lane

Re: [HACKERS] COPY view

From
Bruce Momjian
Date:
Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > It sucks that patches are posted and no action is taken on them for
> > months.  I agree with that.
>
> This particular patch was originally posted during the 8.1 feature
> freeze window (2005-09-29), so it was doomed to a certain amount of
> languishing on the to-worry-about-later list in any case.  We should
> have gotten around to reviewing it sooner than we did (the followup
> discussion was around 2006-06-14), but there was still plenty of time
> at that point to rework it per the discussion and get it into 8.2.
>
> As I see it, we've effectively got a patch that was rejected once,
> and Bruce wants to apply it anyway because no replacement has been
> forthcoming.

Yea, that pretty much sums it up.  Based on the number of people who
wanted it applied, I think we need to have a discussion like this. I can
easily go with rejecting it, but I think the discussion is needed to be
fair to the patch author.

So, what do we want to do with this?  Where did we say we didn't want
SELECT?  I never remember that being discussed.  I remember us saying we
never wanted SELECT or VIEWs because it was going to be slow, but once
the SELECT idea came up, I think we decided we wanted that, and views
could be built on top of that.  I certainly never remember us saying we
didn't want SELECT but wanted views.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] COPY view

From
Robert Treat
Date:
On Tuesday 22 August 2006 16:10, Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > It sucks that patches are posted and no action is taken on them for
> > months.  I agree with that.
>
> This particular patch was originally posted during the 8.1 feature
> freeze window (2005-09-29), so it was doomed to a certain amount of
> languishing on the to-worry-about-later list in any case.  We should
> have gotten around to reviewing it sooner than we did (the followup
> discussion was around 2006-06-14), but there was still plenty of time
> at that point to rework it per the discussion and get it into 8.2.
>
> As I see it, we've effectively got a patch that was rejected once,
> and Bruce wants to apply it anyway because no replacement has been
> forthcoming.
>

Well, unless someone is going to commit to doing it the other way, it seems
the guy who actually codes something offers a better solution than
handwaving... people have also had plenty of time to come up with a
replacement if that's what they really wanted.

--
Robert Treat
Build A Brighter LAMP :: Linux Apache {middleware} PostgreSQL

Re: [HACKERS] COPY view

From
Tom Lane
Date:
Robert Treat <xzilla@users.sourceforge.net> writes:
> On Tuesday 22 August 2006 16:10, Tom Lane wrote:
>> As I see it, we've effectively got a patch that was rejected once,
>> and Bruce wants to apply it anyway because no replacement has been
>> forthcoming.

> Well, unless someone is going to commit to doing it the other way, it seems
> the guy who actually codes something offers a better solution than
> handwaving... people have also had plenty of time to come up with a
> replacement if that's what they really wanted.

The patch submitter has neither provided an updated patch nor defended
his original submission as being the right thing.  If he doesn't take it
seriously enough to have done any followup, why should the rest of us?

At the moment, with the online-index and updatable-views patches both
pretty seriously broken, and no sign that the bitmap-index people are
awake at all, I might take it on myself to fix this one instead of those
others.  But is that what I should be spending my time on in the waning
days of the 8.2 freeze cycle?  Speak now or hold your peace.

            regards, tom lane

Re: [HACKERS] COPY view

From
Bruce Momjian
Date:
Tom Lane wrote:
> Robert Treat <xzilla@users.sourceforge.net> writes:
> > On Tuesday 22 August 2006 16:10, Tom Lane wrote:
> >> As I see it, we've effectively got a patch that was rejected once,
> >> and Bruce wants to apply it anyway because no replacement has been
> >> forthcoming.
>
> > Well, unless someone is going to commit to doing it the other way, it seems
> > the guy who actually codes something offers a better solution than
> > handwaving... people have also had plenty of time to come up with a
> > replacement if that's what they really wanted.
>
> The patch submitter has neither provided an updated patch nor defended
> his original submission as being the right thing.  If he doesn't take it
> seriously enough to have done any followup, why should the rest of us?
>
> At the moment, with the online-index and updatable-views patches both
> pretty seriously broken, and no sign that the bitmap-index people are
> awake at all, I might take it on myself to fix this one instead of those
> others.  But is that what I should be spending my time on in the waning
> days of the 8.2 freeze cycle?  Speak now or hold your peace.

Your analysis is accurate.  You can spend your time on whatever _you_
think is important.  If someone wants to take on COPY VIEW and do all
the work to make it 100%, then they are welcome to do it, but if you
don't feel it is worth it, you can just leave it.  If it isn't 100% by
the time we start beta, it is kept for a later release.

Alvaro has already indicated some problems with the patch (the objection
email is in the patches queue), so it is up to someone to correct at
least that, and if other objections are found, they have to correct
those too before 8.2 beta starts.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] COPY view

From
Böszörményi Zoltán
Date:
Hi,

> Robert Treat <xzilla@users.sourceforge.net> writes:
>> On Tuesday 22 August 2006 16:10, Tom Lane wrote:
>>> As I see it, we've effectively got a patch that was rejected once,
>>> and Bruce wants to apply it anyway because no replacement has been
>>> forthcoming.
>
>> Well, unless someone is going to commit to doing it the other way, it
>> seems
>> the guy who actually codes something offers a better solution than
>> handwaving... people have also had plenty of time to come up with a
>> replacement if that's what they really wanted.
>
> The patch submitter has neither provided an updated patch nor defended
> his original submission as being the right thing.  If he doesn't take it
> seriously enough to have done any followup, why should the rest of us?
>
> At the moment, with the online-index and updatable-views patches both
> pretty seriously broken, and no sign that the bitmap-index people are
> awake at all, I might take it on myself to fix this one instead of those
> others.  But is that what I should be spending my time on in the waning
> days of the 8.2 freeze cycle?  Speak now or hold your peace.
>
>             regards, tom lane

I am willing to get it up to shape and support
both COPY (select) TO and COPY view TO,
the second is rewritten as SELECT * FROM view.
In fact, I already started.

Best regards,
Zoltán Böszörményi


Re: [HACKERS] COPY view

From
Bernd Helmle
Date:

--On Dienstag, August 22, 2006 23:12:21 -0400 Tom Lane <tgl@sss.pgh.pa.us>
wrote:

> At the moment, with the online-index and updatable-views patches both
> pretty seriously broken, and no sign that the bitmap-index people are
> awake at all, I might take it on myself to fix this one instead of those
> others.  But is that what I should be spending my time on in the waning
> days of the 8.2 freeze cycle?  Speak now or hold your peace.

What are these open issues for the updatable views patch you are seeing
exactly?
I'm currently trying to update this patch based on alvaros comments in the
code and
i see the INSERT...RETURNING stuff as the only "big hurd" at the moment
(however, i haven't looked at this closer, but saw your and Jaime's
comments on this...).
It would be nice if we could summarize all open things so everybody who is
able to work on this gets a complete overview.

--
  Thanks

                    Bernd

Re: [HACKERS] COPY view

From
Karel Zak
Date:
On Tue, Aug 22, 2006 at 01:11:22PM -0400, Andrew Dunstan wrote:
> There's nothing hidden (unless it's also hidden from me ;-) )
>
> I take it that when you talk about "we did this" you are referring to
> the patch from Karel Zak.

 Hans has been original author of COPY VIEW idea and I've wrote it for
 his customer (yes, it was sponsored work).

    Karel

--
 Karel Zak  <kzak@redhat.com>

Leaving... (was: Re: [HACKERS] COPY view)

From
Karel Zak
Date:
 Hi all,

 seriously... I don't have time to work on PostgreSQL. It's time to
 say that I'm leaving this project. So, if you found some my broken
 code or whatever in PostgreSQL you should go and fix it. It's
 community-driven project. It's about collaboration -- don't ask "why
 should I help" -- go and help!

 It was nice time and really big experience, but in the world is more
 projects and many of them need more help than already stable (do you
 remember PostgreSQL 6.5? :-) and very reliable PostgreSQL.

 Good bye!
    Karel

On Tue, Aug 22, 2006 at 11:12:21PM -0400, Tom Lane wrote:
> The patch submitter has neither provided an updated patch nor defended
> his original submission as being the right thing.  If he doesn't take it
> seriously enough to have done any followup, why should the rest of us?

--
 Karel Zak  <kzak@redhat.com>

Re: [HACKERS] COPY view

From
Andrew Dunstan
Date:

Tom Lane wrote:
>
> At the moment, with the online-index and updatable-views patches both
> pretty seriously broken, and no sign that the bitmap-index people are
> awake at all, I might take it on myself to fix this one instead of those
> others.  But is that what I should be spending my time on in the waning
> days of the 8.2 freeze cycle?  Speak now or hold your peace.
>
>
>

Personally, I would say that this is less important than updatable views
but more than  online indexes. If it could be fixed just for the view
case in a day or so then I think it's worth it.

cheers

andrew

Re: [HACKERS] COPY view

From
Tom Lane
Date:
Bernd Helmle <mailings@oopsware.de> writes:
> What are these open issues for the updatable views patch you are seeing
> exactly?

Didn't Alvaro list a bunch of issues when he put the patch back up for
comment?  I have not looked at it myself yet.

> i see the INSERT...RETURNING stuff as the only "big hurd" at the moment

That's not the fault of the updatable-views patch, but it definitely is
something we need to put some time into :-(

            regards, tom lane

Re: [HACKERS] COPY view

From
Bernd Helmle
Date:

--On Mittwoch, August 23, 2006 08:24:55 -0400 Tom Lane <tgl@sss.pgh.pa.us>
wrote:

>> What are these open issues for the updatable views patch you are seeing
>> exactly?
>
> Didn't Alvaro list a bunch of issues when he put the patch back up for
> comment?  I have not looked at it myself yet.

Indeed he did and this helps a lot to clean up some parts of the code (oh,
thanks
to him for reviewing this, i think i forgot that :( ). I thought you were
refering to
some specific showstoppers i've missed.

--
  Thanks

                    Bernd

Re: [HACKERS] COPY view

From
Böszörményi Zoltán
Date:
Hi,

> Tom Lane wrote:
>>
>> At the moment, with the online-index and updatable-views patches both
>> pretty seriously broken, and no sign that the bitmap-index people are
>> awake at all, I might take it on myself to fix this one instead of those
>> others.  But is that what I should be spending my time on in the waning
>> days of the 8.2 freeze cycle?  Speak now or hold your peace.
>>
>>
>>
>
> Personally, I would say that this is less important than updatable views
> but more than  online indexes. If it could be fixed just for the view
> case in a day or so then I think it's worth it.
>
> cheers
>
> andrew

It seems I was able to get it working for both the VIEW and SELECT
cases. I still have one issue, the reference to the select is left open
and it complains on closing the transaction. But basically works.

Best regards,
Zoltán Böszörményi


Re: [HACKERS] COPY view

From
Andrew Dunstan
Date:
Böszörményi Zoltán wrote:
> Hi,
>
>
>> Tom Lane wrote:
>>
>>> At the moment, with the online-index and updatable-views patches both
>>> pretty seriously broken, and no sign that the bitmap-index people are
>>> awake at all, I might take it on myself to fix this one instead of those
>>> others.  But is that what I should be spending my time on in the waning
>>> days of the 8.2 freeze cycle?  Speak now or hold your peace.
>>>
>>>
>>>
>>>
>> Personally, I would say that this is less important than updatable views
>> but more than  online indexes. If it could be fixed just for the view
>> case in a day or so then I think it's worth it.
>>
>> cheers
>>
>> andrew
>>
>
> It seems I was able to get it working for both the VIEW and SELECT
> cases. I still have one issue, the reference to the select is left open
> and it complains on closing the transaction. But basically works.
>
>
>

So when will you send in a revised patch?

cheers

andrew


Re: [HACKERS] COPY view

From
Böszörményi Zoltán
Date:
> Böszörményi Zoltán wrote:
>> Hi,
>>
>>
>>> Tom Lane wrote:
>>>
>>>> At the moment, with the online-index and updatable-views patches both
>>>> pretty seriously broken, and no sign that the bitmap-index people are
>>>> awake at all, I might take it on myself to fix this one instead of
>>>> those
>>>> others.  But is that what I should be spending my time on in the
>>>> waning
>>>> days of the 8.2 freeze cycle?  Speak now or hold your peace.
>>>>
>>>>
>>>>
>>>>
>>> Personally, I would say that this is less important than updatable
>>> views
>>> but more than  online indexes. If it could be fixed just for the view
>>> case in a day or so then I think it's worth it.
>>>
>>> cheers
>>>
>>> andrew
>>>
>>
>> It seems I was able to get it working for both the VIEW and SELECT
>> cases. I still have one issue, the reference to the select is left open
>> and it complains on closing the transaction. But basically works.
>>
>>
>>
>
> So when will you send in a revised patch?
>
> cheers
>
> andrew

Soon. :-)

Best regards,
Zoltán Böszörményi


Re: [HACKERS] COPY view

From
Alvaro Herrera
Date:
Böszörményi Zoltán wrote:

> > So when will you send in a revised patch?
>
> Soon. :-)

No, don't send it "soon".  We're in feature freeze already (and have
been for three weeks).  You need to send it now.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: [HACKERS] COPY view

From
Böszörményi Zoltán
Date:
> Böszörményi Zoltán wrote:
>
>> > So when will you send in a revised patch?
>>
>> Soon. :-)
>
> No, don't send it "soon".  We're in feature freeze already (and have
> been for three weeks).  You need to send it now.

I have to test it some more but I will send it.

Best regards,
Zoltán Böszörményi


Re: [HACKERS] COPY view

From
Bruce Momjian
Date:
B�sz�rm�nyi Zolt�n wrote:
> > B?sz?rm?nyi Zolt?n wrote:
> >
> >> > So when will you send in a revised patch?
> >>
> >> Soon. :-)
> >
> > No, don't send it "soon".  We're in feature freeze already (and have
> > been for three weeks).  You need to send it now.
>
> I have to test it some more but I will send it.

I think Alvaro is saying we need it in a few days, not longer.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] COPY view

From
Böszörményi Zoltán
Date:
> Böszörményi Zoltán wrote:
>> > B?sz?rm?nyi Zolt?n wrote:
>> >
>> >> > So when will you send in a revised patch?
>> >>
>> >> Soon. :-)
>> >
>> > No, don't send it "soon".  We're in feature freeze already (and have
>> > been for three weeks).  You need to send it now.
>>
>> I have to test it some more but I will send it.
>
> I think Alvaro is saying we need it in a few days, not longer.

Of course.

Best regards,
Zoltán Böszörményi


Re: [HACKERS] COPY view

From
Andrew Dunstan
Date:
Bruce Momjian wrote:
> I think Alvaro is saying we need it in a few days, not longer.
>
>

I thought he was saying today ;-)

cheers

andrew


Re: [HACKERS] COPY view

From
Bruce Momjian
Date:
Andrew Dunstan wrote:
> Bruce Momjian wrote:
> > I think Alvaro is saying we need it in a few days, not longer.
> >
> >
>
> I thought he was saying today ;-)

He actually said "now", but I don't think we need it immediately,
especially if he is still working on it.  We are at least 1-2 weeks away
from having all open patches applied.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] COPY view

From
Alvaro Herrera
Date:
Bruce Momjian wrote:
> Andrew Dunstan wrote:
> > Bruce Momjian wrote:
> > > I think Alvaro is saying we need it in a few days, not longer.
> >
> > I thought he was saying today ;-)
>
> He actually said "now", but I don't think we need it immediately,
> especially if he is still working on it.  We are at least 1-2 weeks away
> from having all open patches applied.

Yes, I'm saying today so that we can all look at it and point obvious
mistakes now, not in 2 weeks from now.  Release early, release often.
If the patch contains a mistake and we find out in 2 weeks, are we going
to fix it?  No, we are going to reject it.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: [HACKERS] COPY view

From
Bruce Momjian
Date:
Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > Andrew Dunstan wrote:
> > > Bruce Momjian wrote:
> > > > I think Alvaro is saying we need it in a few days, not longer.
> > >
> > > I thought he was saying today ;-)
> >
> > He actually said "now", but I don't think we need it immediately,
> > especially if he is still working on it.  We are at least 1-2 weeks away
> > from having all open patches applied.
>
> Yes, I'm saying today so that we can all look at it and point obvious
> mistakes now, not in 2 weeks from now.  Release early, release often.
> If the patch contains a mistake and we find out in 2 weeks, are we going
> to fix it?  No, we are going to reject it.

OK, I understand.   B?sz?rm?nyi, post now so we can see where you are,
but keep working and send it to us again when you are done.  No sense in
not posting your working version.

--
  Bruce Momjian   bruce@momjian.us
  EnterpriseDB    http://www.enterprisedb.com

  + If your life is a hard drive, Christ can be your backup. +

Re: [HACKERS] COPY view

From
Zoltan Boszormenyi
Date:
Hi,

Bruce Momjian írta:
> Alvaro Herrera wrote:
>
>> Bruce Momjian wrote:
>>
>>> Andrew Dunstan wrote:
>>>
>>>> Bruce Momjian wrote:
>>>>
>>>>> I think Alvaro is saying we need it in a few days, not longer.
>>>>>
>>>> I thought he was saying today ;-)
>>>>
>>> He actually said "now", but I don't think we need it immediately,
>>> especially if he is still working on it.  We are at least 1-2 weeks away
>>> from having all open patches applied.
>>>
>> Yes, I'm saying today so that we can all look at it and point obvious
>> mistakes now, not in 2 weeks from now.  Release early, release often.
>> If the patch contains a mistake and we find out in 2 weeks, are we going
>> to fix it?  No, we are going to reject it.
>>
>
> OK, I understand.   B?sz?rm?nyi, post now so we can see where you are,
> but keep working and send it to us again when you are done.  No sense in
> not posting your working version.
>

OK, here's my current version. The reference leak is fixed.
But as my testcase shows, it only works for single selects
currently. The parser accepts it but COPY doesn't produce
the expected output. Please, suggest a solution.

BTW, my first name is Zoltán.

Best regards,
Zoltán Böszörményi


Attachment

Re: [HACKERS] COPY view

From
Zoltan Boszormenyi
Date:
Zoltan Boszormenyi írta:
> Hi,
>
> Bruce Momjian írta:
>> Alvaro Herrera wrote:
>>
>>> Bruce Momjian wrote:
>>>
>>>> Andrew Dunstan wrote:
>>>>
>>>>> Bruce Momjian wrote:
>>>>>
>>>>>> I think Alvaro is saying we need it in a few days, not longer.
>>>>>>
>>>>> I thought he was saying today ;-)
>>>>>
>>>> He actually said "now", but I don't think we need it immediately,
>>>> especially if he is still working on it.  We are at least 1-2 weeks
>>>> away
>>>> from having all open patches applied.
>>>>
>>> Yes, I'm saying today so that we can all look at it and point obvious
>>> mistakes now, not in 2 weeks from now.  Release early, release often.
>>> If the patch contains a mistake and we find out in 2 weeks, are we
>>> going
>>> to fix it?  No, we are going to reject it.
>>>
>>
>> OK, I understand.   B?sz?rm?nyi, post now so we can see where you are,
>> but keep working and send it to us again when you are done.  No sense in
>> not posting your working version.
>>
>
> OK, here's my current version. The reference leak is fixed.
> But as my testcase shows, it only works for single selects
> currently. The parser accepts it but COPY doesn't produce
> the expected output. Please, suggest a solution.

I meant that UNION selects, subselects don't work yet.


>
> BTW, my first name is Zoltán.
>
> Best regards,
> Zoltán Böszörményi
>
> ------------------------------------------------------------------------
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faq
>


Re: [HACKERS] COPY view

From
Alvaro Herrera
Date:
Zoltan Boszormenyi wrote:

> OK, here's my current version. The reference leak is fixed.
> But as my testcase shows, it only works for single selects
> currently. The parser accepts it but COPY doesn't produce
> the expected output. Please, suggest a solution.

I'm not sure I agree with the approach of creating a fake "SELECT * FROM
foo" in analyze.c in the relation case and passing it back to the parser
to create a Query node.  That's not there in the original code and you
shouldn't need it.  Just let the case where COPY gets a relation
continue to handle it as it does today, and add a separate case for the
SELECT.

That doesn't help you with the UNION stuff though.

--
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Re: [HACKERS] COPY view

From
Zoltan Boszormenyi
Date:
Alvaro Herrera írta:
> Zoltan Boszormenyi wrote:
>
>
>> OK, here's my current version. The reference leak is fixed.
>> But as my testcase shows, it only works for single selects
>> currently. The parser accepts it but COPY doesn't produce
>> the expected output. Please, suggest a solution.
>>
>
> I'm not sure I agree with the approach of creating a fake "SELECT * FROM
> foo" in analyze.c in the relation case and passing it back to the parser
> to create a Query node.  That's not there in the original code and you
> shouldn't need it.  Just let the case where COPY gets a relation
> continue to handle it as it does today, and add a separate case for the
> SELECT.
>

The exact same code was there,
e.g. parse and rewrite "SELECT * FROM view"
just not in analyze.c. I will try without it, though.


> That doesn't help you with the UNION stuff though.
>

:-(


Re: [HACKERS] COPY view

From
Zoltan Boszormenyi
Date:
Tom Lane írta:
> Zoltan Boszormenyi <zboszor@dunaweb.hu> writes:
>
>> How about the callback solution for the SELECT case
>> that was copied from the original? Should I consider
>> open-coding in copy.c what ExecutorRun() does
>> to avoid the callback?
>>
>
> Adding a DestReceiver type is a good solution ... although that static
> variable is not.  Instead define a DestReceiver extension struct that
> can carry the CopyState pointer for you.

Done.

>   You could also consider
> putting the copy-from-view-specific state fields into DestReceiver
> instead of CopyState, though this is a bit asymmetric with the relation
> case so maybe it's not really cleaner.
>

Left it alone for now.

> BTW, lose the tuple_to_values function --- it's an extremely bad
> reimplementation of heap_deform_tuple.

Done.

>   copy_dest_printtup also seems
> coded without regard for the TupleTableSlot access API (read printtup()
> to see what to do instead).

I am still interpreting it. Can you give me some hints
besides using slot_getallattrs(slot)?

>   And what's the point of factoring out the
> heap_getnext loop as CopyRelationTo?  It's not like that lets you share
> any more code.  The inside of the loop, ie what you've called
> CopyValuesTo, is the sharable part.
>

Done.

The option parsing and error checking is now common.

I also changed it to use transformStmt() in analyze.c.
However, both the UNION and sunselect cases give me
something like this:

ERROR:  could not open relation 1663/16384/16723: No such file or directory

What else can I do with it?

Best regards,
Zoltán Böszörményi


Attachment

Re: [HACKERS] COPY view

From
Zoltan Boszormenyi
Date:
Zoltan Boszormenyi írta:
> The option parsing and error checking is now common.
>
> I also changed it to use transformStmt() in analyze.c.
> However, both the UNION and sunselect cases give me
> something like this:
>
> ERROR:  could not open relation 1663/16384/16723: No such file or
> directory
>
> What else can I do with it?

But a single SELECT with two tables joined
also works so it must be something trivial.

Best regards,
Zoltán Böszörményi


Re: [HACKERS] COPY view

From
Zoltan Boszormenyi
Date:
Zoltan Boszormenyi írta:
> Zoltan Boszormenyi írta:
>> The option parsing and error checking is now common.
>>
>> I also changed it to use transformStmt() in analyze.c.
>> However, both the UNION and sunselect cases give me
>> something like this:
>>
>> ERROR:  could not open relation 1663/16384/16723: No such file or
>> directory
>>
>> What else can I do with it?
>
> But a single SELECT with two tables joined
> also works so it must be something trivial.

Now UNIONs and subselects also work.

Your concern about copy_dest_printtup()
wasn't solved yet.

Best regards,
Zoltán Böszörményi



Attachment