Обсуждение: Fixed PPAS trigger column view condition
Hi Dave,
There is one issue with pgadmin while viewing the trigger definition created for PPAS server as below:
CREATE OR REPLACE TRIGGER tr_a
BEFORE UPDATE OF cl1 ON a FOR EACH ROW
begin
if(:new.cl1 < 0) then
insert into a values (:new.*);
END if;
END;
after creating the trigger using above query,column name cl1(highlighted part above) is not visible in property window for that trigger.
In current implementation, there is a condition:
GetLanguage() != wxT("edbspl") in
ShowTreeDetail() and ReadColumnDetails() functions in pgTrigger.cpp file which will prevent it to fetch column name in case db server is PPAS.
I have removed that condition and tested it. It works fine now.
I have attached the same patch with this mail for your reference.
Please review it and if it looks good to you please do the needful.
Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb
Вложения
On Fri, Oct 10, 2014 at 1:35 PM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote: > Hi Dave, > > There is one issue with pgadmin while viewing the trigger definition created > for PPAS server as below: > > CREATE OR REPLACE TRIGGER tr_a > BEFORE UPDATE OF cl1 ON a FOR EACH ROW > begin > if(:new.cl1 < 0) then > insert into a values (:new.*); > END if; > END; > > after creating the trigger using above query,column name cl1(highlighted > part above) is not visible in property window for that trigger. > > In current implementation, there is a condition: > > GetLanguage() != wxT("edbspl") in > > ShowTreeDetail() and ReadColumnDetails() functions in pgTrigger.cpp file > which will prevent it to fetch column name in case db server is PPAS. > > I have removed that condition and tested it. It works fine now. > I have attached the same patch with this mail for your reference. > Please review it and if it looks good to you please do the needful. Are per-column triggers in PPAS something that's been added in a recent release? I wonder if there should be a version check in there, as the code was pretty clearly intended to be written as it is originally. -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Hi Dave,
per trigger column is available in PPAS since 8.5 (9.0) version.
It seems that it was wrongly assume by "Guillaume Lelarge" who has added this check in pgadmin that is should not support per column trigger for PPAS.
So the current patch is working for PG and PPAS both.
Please let me know if anything is missing.
Regards,
Sanket Mehta
Sr Software engineer
Enterprisedb
On Fri, Oct 10, 2014 at 9:24 PM, Dave Page <dpage@pgadmin.org> wrote:
Are per-column triggers in PPAS something that's been added in aOn Fri, Oct 10, 2014 at 1:35 PM, Sanket Mehta
<sanket.mehta@enterprisedb.com> wrote:
> Hi Dave,
>
> There is one issue with pgadmin while viewing the trigger definition created
> for PPAS server as below:
>
> CREATE OR REPLACE TRIGGER tr_a
> BEFORE UPDATE OF cl1 ON a FOR EACH ROW
> begin
> if(:new.cl1 < 0) then
> insert into a values (:new.*);
> END if;
> END;
>
> after creating the trigger using above query,column name cl1(highlighted
> part above) is not visible in property window for that trigger.
>
> In current implementation, there is a condition:
>
> GetLanguage() != wxT("edbspl") in
>
> ShowTreeDetail() and ReadColumnDetails() functions in pgTrigger.cpp file
> which will prevent it to fetch column name in case db server is PPAS.
>
> I have removed that condition and tested it. It works fine now.
> I have attached the same patch with this mail for your reference.
> Please review it and if it looks good to you please do the needful.
recent release? I wonder if there should be a version check in there,
as the code was pretty clearly intended to be written as it is
originally.
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
OK, thanks. Ashesh, please review and commit - thanks! On Mon, Oct 13, 2014 at 10:04 AM, Sanket Mehta <sanket.mehta@enterprisedb.com> wrote: > Hi Dave, > > per trigger column is available in PPAS since 8.5 (9.0) version. > It seems that it was wrongly assume by "Guillaume Lelarge" who has added > this check in pgadmin that is should not support per column trigger for > PPAS. > So the current patch is working for PG and PPAS both. > > Please let me know if anything is missing. > > > Regards, > Sanket Mehta > Sr Software engineer > Enterprisedb > > On Fri, Oct 10, 2014 at 9:24 PM, Dave Page <dpage@pgadmin.org> wrote: >> >> On Fri, Oct 10, 2014 at 1:35 PM, Sanket Mehta >> <sanket.mehta@enterprisedb.com> wrote: >> > Hi Dave, >> > >> > There is one issue with pgadmin while viewing the trigger definition >> > created >> > for PPAS server as below: >> > >> > CREATE OR REPLACE TRIGGER tr_a >> > BEFORE UPDATE OF cl1 ON a FOR EACH ROW >> > begin >> > if(:new.cl1 < 0) then >> > insert into a values (:new.*); >> > END if; >> > END; >> > >> > after creating the trigger using above query,column name cl1(highlighted >> > part above) is not visible in property window for that trigger. >> > >> > In current implementation, there is a condition: >> > >> > GetLanguage() != wxT("edbspl") in >> > >> > ShowTreeDetail() and ReadColumnDetails() functions in pgTrigger.cpp >> > file >> > which will prevent it to fetch column name in case db server is PPAS. >> > >> > I have removed that condition and tested it. It works fine now. >> > I have attached the same patch with this mail for your reference. >> > Please review it and if it looks good to you please do the needful. >> >> Are per-column triggers in PPAS something that's been added in a >> recent release? I wonder if there should be a version check in there, >> as the code was pretty clearly intended to be written as it is >> originally. >> >> >> -- >> Dave Page >> Blog: http://pgsnake.blogspot.com >> Twitter: @pgsnake >> >> EnterpriseDB UK: http://www.enterprisedb.com >> The Enterprise PostgreSQL Company > > -- Dave Page Blog: http://pgsnake.blogspot.com Twitter: @pgsnake EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Mon, Oct 13, 2014 at 2:35 PM, Dave Page <dpage@pgadmin.org> wrote:
OK, thanks. Ashesh, please review and commit - thanks!
On Mon, Oct 13, 2014 at 10:04 AM, Sanket Mehta<sanket.mehta@enterprisedb.com> wrote:
> Hi Dave,
>
> per trigger column is available in PPAS since 8.5 (9.0) version.
> It seems that it was wrongly assume by "Guillaume Lelarge" who has added
> this check in pgadmin that is should not support per column trigger for
> PPAS.
> So the current patch is working for PG and PPAS both.
>
> Please let me know if anything is missing.
>
>
> Regards,
> Sanket Mehta
> Sr Software engineer
> Enterprisedb
>
> On Fri, Oct 10, 2014 at 9:24 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Fri, Oct 10, 2014 at 1:35 PM, Sanket Mehta
>> <sanket.mehta@enterprisedb.com> wrote:
>> > Hi Dave,
>> >
>> > There is one issue with pgadmin while viewing the trigger definition
>> > created
>> > for PPAS server as below:
>> >
>> > CREATE OR REPLACE TRIGGER tr_a
>> > BEFORE UPDATE OF cl1 ON a FOR EACH ROW
>> > begin
>> > if(:new.cl1 < 0) then
>> > insert into a values (:new.*);
>> > END if;
>> > END;
>> >
>> > after creating the trigger using above query,column name cl1(highlighted
>> > part above) is not visible in property window for that trigger.
>> >
>> > In current implementation, there is a condition:
>> >
>> > GetLanguage() != wxT("edbspl") in
>> >
>> > ShowTreeDetail() and ReadColumnDetails() functions in pgTrigger.cpp
>> > file
>> > which will prevent it to fetch column name in case db server is PPAS.
>> >
>> > I have removed that condition and tested it. It works fine now.
>> > I have attached the same patch with this mail for your reference.
>> > Please review it and if it looks good to you please do the needful.
>>
>> Are per-column triggers in PPAS something that's been added in a
>> recent release? I wonder if there should be a version check in there,
>> as the code was pretty clearly intended to be written as it is
>> originally.
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>
--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake
EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company