Re: [HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-10 Thread Haribabu Kommi 
On Fri, Nov 11, 2016 at 6:15 AM, Tom Lane  wrote:

> Haribabu Kommi  writes:
> > [ copy_to_view_3.patch ]
>
> Pushed with cosmetic adjustments.
>

Thank you.


Regards,
Hari Babu
Fujitsu Australia

Re: [HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-10 Thread Tom Lane 
Haribabu Kommi  writes:
> [ copy_to_view_3.patch ]

Pushed with cosmetic adjustments.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-07 Thread Dilip Kumar 
On Fri, Nov 4, 2016 at 7:14 AM, Haribabu Kommi  wrote:
>> + if (cstate->rel->rd_rel->relkind != RELKIND_RELATION
>> + && (!cstate->rel->trigdesc ||
>> + !cstate->rel->trigdesc->trig_insert_instead_row))
>
>
> changed.
>
>>
>> Meanwhile I will test it and give the feedback.
>
>
> Thanks.
>
> Updated patch is attached with added regression tests.

I have done with review and test, patch looks fine to me.
moved to "Ready for Committer"

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-03 Thread Haribabu Kommi 
On Thu, Nov 3, 2016 at 5:23 PM, Dilip Kumar  wrote:

> On Thu, Nov 3, 2016 at 9:57 AM, Haribabu Kommi 
> wrote:
> > Thanks for your suggestion. Yes, I agree that adding a hint is good.
> > Updated patch is attached with addition of hint message.
> >
> > 2016-11-03 14:56:28.685 AEDT [7822] ERROR:  cannot copy to view "ttt_v"
> > 2016-11-03 14:56:28.685 AEDT [7822] HINT:  To enable copy to view,
> provide
> > an INSTEAD OF INSERT trigger
> > 2016-11-03 14:56:28.685 AEDT [7822] STATEMENT:  COPY ttt_v FROM stdin;
>
> Okay, Patch in general looks fine to me. One cosmetic comments, IMHO
> in PG we follow operator at end of the line, so move '&&' to end of
> the previous line.
>
> + if (cstate->rel->rd_rel->relkind != RELKIND_RELATION
> + && (!cstate->rel->trigdesc ||
> + !cstate->rel->trigdesc->trig_insert_instead_row))
>

changed.


> Meanwhile I will test it and give the feedback.


Thanks.

Updated patch is attached with added regression tests.

Regards,
Hari Babu
Fujitsu Australia


copy_to_view_3.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-03 Thread Dilip Kumar 
On Thu, Nov 3, 2016 at 9:57 AM, Haribabu Kommi  wrote:
> Thanks for your suggestion. Yes, I agree that adding a hint is good.
> Updated patch is attached with addition of hint message.
>
> 2016-11-03 14:56:28.685 AEDT [7822] ERROR:  cannot copy to view "ttt_v"
> 2016-11-03 14:56:28.685 AEDT [7822] HINT:  To enable copy to view, provide
> an INSTEAD OF INSERT trigger
> 2016-11-03 14:56:28.685 AEDT [7822] STATEMENT:  COPY ttt_v FROM stdin;

Okay, Patch in general looks fine to me. One cosmetic comments, IMHO
in PG we follow operator at end of the line, so move '&&' to end of
the previous line.

+ if (cstate->rel->rd_rel->relkind != RELKIND_RELATION
+ && (!cstate->rel->trigdesc ||
+ !cstate->rel->trigdesc->trig_insert_instead_row))

Meanwhile I will test it and give the feedback.

-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-02 Thread Haribabu Kommi 
On Tue, Nov 1, 2016 at 8:05 PM, Dilip Kumar  wrote:

> On Tue, Nov 1, 2016 at 12:40 PM, Haribabu Kommi
>  wrote:
> > COPY command is treated as an UTILITY command, During the query
> > processing, the rules are not applied on the COPY command, and in the
> > execution of COPY command, it just inserts the data into the heap and
> > indexes with direct calls.
> >
> > I feel supporting the COPY command for the case-2 is little bit complex
> > and may not be that much worth.
>
> I agree it will be complex..
> >
> > Attached is the updated patch with doc changes.
>
> Now since we are adding support for INSTEAD OF TRIGGER in COPY FROM
> command, It will be good that we provide a HINT to user if INSTEAD of
> trigger does not exist, like we do in case of insert ?
>
> INSERT case:
> postgres=# insert into ttt_v values(7);
> 2016-11-01 14:31:39.845 IST [72343] ERROR:  cannot insert into view "ttt_v"
> 2016-11-01 14:31:39.845 IST [72343] DETAIL:  Views that do not select
> from a single table or view are not automatically updatable.
> 2016-11-01 14:31:39.845 IST [72343] HINT:  To enable inserting into
> the view, provide an INSTEAD OF INSERT trigger or an unconditional ON
> INSERT DO INSTEAD rule.
>
> COPY case:
> postgres=# COPY ttt_v FROM stdin;
> 2016-11-01 14:31:52.235 IST [72343] ERROR:  cannot copy to view "ttt_v"
> 2016-11-01 14:31:52.235 IST [72343] STATEMENT:  COPY ttt_v FROM stdin;


Thanks for your suggestion. Yes, I agree that adding a hint is good.
Updated patch is attached with addition of hint message.

2016-11-03 14:56:28.685 AEDT [7822] ERROR:  cannot copy to view "ttt_v"
2016-11-03 14:56:28.685 AEDT [7822] HINT:  To enable copy to view, provide
an INSTEAD OF INSERT trigger
2016-11-03 14:56:28.685 AEDT [7822] STATEMENT:  COPY ttt_v FROM stdin;


Regards,
Hari Babu
Fujitsu Australia


copy_to_view_2.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-01 Thread Dilip Kumar 
On Tue, Nov 1, 2016 at 12:40 PM, Haribabu Kommi
 wrote:
> COPY command is treated as an UTILITY command, During the query
> processing, the rules are not applied on the COPY command, and in the
> execution of COPY command, it just inserts the data into the heap and
> indexes with direct calls.
>
> I feel supporting the COPY command for the case-2 is little bit complex
> and may not be that much worth.

I agree it will be complex..
>
> Attached is the updated patch with doc changes.

Now since we are adding support for INSTEAD OF TRIGGER in COPY FROM
command, It will be good that we provide a HINT to user if INSTEAD of
trigger does not exist, like we do in case of insert ?

INSERT case:
postgres=# insert into ttt_v values(7);
2016-11-01 14:31:39.845 IST [72343] ERROR:  cannot insert into view "ttt_v"
2016-11-01 14:31:39.845 IST [72343] DETAIL:  Views that do not select
from a single table or view are not automatically updatable.
2016-11-01 14:31:39.845 IST [72343] HINT:  To enable inserting into
the view, provide an INSTEAD OF INSERT trigger or an unconditional ON
INSERT DO INSTEAD rule.

COPY case:
postgres=# COPY ttt_v FROM stdin;
2016-11-01 14:31:52.235 IST [72343] ERROR:  cannot copy to view "ttt_v"
2016-11-01 14:31:52.235 IST [72343] STATEMENT:  COPY ttt_v FROM stdin;


-- 
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] Re: [BUGS] BUG #14350: VIEW with INSTEAD OF INSERT TRIGGER and COPY. Missing feature or working as designed.

2016-11-01 Thread Haribabu Kommi 
On Tue, Nov 1, 2016 at 3:54 PM, Dilip Kumar  wrote:

> On Tue, Oct 4, 2016 at 1:07 PM, Haribabu Kommi 
> wrote:
> > I think currently there is no handling of INSTEAD of triggers in the copy
> > functionality.
> >
> > It didn't seem difficult to the support the same, until unless there are
> any
> > problems for complext queries, so after adding the INSTEAD of triggers
> > check and calling the ExecIRInsertTriggers function, the Copy is also
> > working for the view.
> >
> > Attached is a POC patch of the same. I didn't checked all the possible
> > scenarios.
>
> We support insert into view in below 2 cases..
>
> 1. INSTEAD OF INSERT trigger
> 2. or an unconditional ON INSERT DO INSTEAD rule
>

Yes, I agree that the above are the two cases where the insert is possible
on a view other than updatable views.



> In your patch we are supporting first one in COPY command, Will it not
> be good to support second one also in COPY command ?
>


COPY command is treated as an UTILITY command, During the query
processing, the rules are not applied on the COPY command, and in the
execution of COPY command, it just inserts the data into the heap and
indexes with direct calls.

I feel supporting the COPY command for the case-2 is little bit complex
and may not be that much worth.

Attached is the updated patch with doc changes.

Regards,
Hari Babu
Fujitsu Australia


copy_to_view_1.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers