Re: [pgadmin-hackers] potential invalid input field of pgAdmin New Column GUI

Hi Sanket

Patch looks good to me. I have tested couple of scenarios and it works fine.

On Wed, Nov 26, 2014 at 3:27 PM, Sanket Mehta <sanket(dot)mehta(at)enterprisedb(dot)com
> wrote:

> Hi Akshay,
>
>
> On Tue, Nov 25, 2014 at 2:57 PM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> Hi Sanket
>>
>> On Mon, Nov 24, 2014 at 5:18 PM, Sanket Mehta <
>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>
>>> Hi Akshay,
>>>
>>>
>>> On Wed, Nov 19, 2014 at 4:39 PM, Akshay Joshi <
>>> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>>>
>>>> Hi Sanket
>>>>
>>>> Below are the file wise review comments
>>>>
>>>>
>>>> 1. ctlSeclabelPanel.cpp and ctlSeclabelPanel.h
>>>> - Correct the function name "GetCUrrentProviderLabelArray"
>>>> according to camel case.
>>>> - Use trim before check the empty condition of txtProvider and
>>>> txtSeclabel used in ctlSeclabelPanel::OnChange().
>>>> 2. dlgTable.cpp
>>>> - Security label sql not removed from SQL pane of dlgTable even
>>>> though I have removed the column.
>>>> - Security label sql gets overwritten when I add more then one
>>>> column.
>>>> - Changes are overwritten in the SQL pane of dlgTable when I
>>>> have changed the attributes of the column more then once by clicking on
>>>> "Change" button.
>>>>
>>>> Above Issue is still reproducible.
>>
> All the reproducible issues are resolved.
>
>>
>>>> 1. dlgColumn.cpp
>>>> - Remove comment //code removed for testing from constructor if
>>>> testing is done.
>>>>
>>>> Above issues are resolved.
>>> PFA the revised patch.
>>>
>>
>> I have found one more issue when we add security label while creating
>> a new table it throws syntax error.
>>
> This issue is also resolved.
>
> PFA the revised patch.
>
>>
>>>
>>>> On Tue, Nov 18, 2014 at 6:10 PM, Sanket Mehta <
>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>
>>>> Hi,
>>>>>
>>>>> Below are the thing I have taken care in this patch:
>>>>>
>>>>> 1. While creating the new table using table wizard, when any
>>>>> variables or security labels are specified for certain column, those
>>>>> were not visible on *sql tab* of table wizard.
>>>>> 2. when new/existing column dialog is open from table dialog then
>>>>> privileges and security labels in column dialog are not being persistent
>>>>> for new changes.
>>>>>
>>>>>
>>>>> Regards,
>>>>> Sanket Mehta
>>>>> Sr Software engineer
>>>>> Enterprisedb
>>>>>
>>>>> On Tue, Nov 18, 2014 at 5:54 PM, Sanket Mehta <
>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>
>>>>>> Hi Akshay,
>>>>>>
>>>>>> PFA the patch.
>>>>>> Please review it and let me know if anything is missing.
>>>>>>
>>>>>> Regards,
>>>>>> Sanket Mehta
>>>>>> Sr Software engineer
>>>>>> Enterprisedb
>>>>>>
>>>>>> On Tue, Nov 11, 2014 at 7:21 PM, Ashesh Vashi <
>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>
>>>>>>> Hi Sanket,
>>>>>>>
>>>>>>> Apart from variable persistence issue taken care in your patch, I've
>>>>>>> also observed the data persistence issue with:
>>>>>>> (When reloading the existing/new column in column dialog from table
>>>>>>> dialog)
>>>>>>> 1. Priviledges
>>>>>>> 2. Security Lables
>>>>>>>
>>>>>>> I also observed, when I remove some privileges from an existing
>>>>>>> column, it generates SQL like, it needs to remove that column first, and
>>>>>>> then add that column, and the modify the new privileges.
>>>>>>> Can you also look into it?
>>>>>>>
>>>>>>> --
>>>>>>>
>>>>>>> Thanks & Regards,
>>>>>>>
>>>>>>> Ashesh Vashi
>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>> <http://www.enterprisedb.com>
>>>>>>>
>>>>>>>
>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>
>>>>>>> On Tue, Nov 11, 2014 at 7:06 PM, Ashesh Vashi <
>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>
>>>>>>>> Hi Sanket,
>>>>>>>>
>>>>>>>> Quick review suggests that, you're not following the consistent
>>>>>>>> name convention in new functions as per pgAdmin3 coding standard.
>>>>>>>> I also observed a white-space warning, while apply the patch.
>>>>>>>>
>>>>>>>> Please resend the patch after resolving these issues.
>>>>>>>>
>>>>>>>> --
>>>>>>>>
>>>>>>>> Thanks & Regards,
>>>>>>>>
>>>>>>>> Ashesh Vashi
>>>>>>>> EnterpriseDB INDIA: Enterprise PostgreSQL Company
>>>>>>>> <http://www.enterprisedb.com>
>>>>>>>>
>>>>>>>>
>>>>>>>> *http://www.linkedin.com/in/asheshvashi*
>>>>>>>> <http://www.linkedin.com/in/asheshvashi>
>>>>>>>>
>>>>>>>> On Tue, Nov 11, 2014 at 5:25 PM, Sanket Mehta <
>>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>>
>>>>>>>>> Hi,
>>>>>>>>>
>>>>>>>>> Issue was occurring as mentioned below:
>>>>>>>>>
>>>>>>>>> While creating the new table using table wizard, when any
>>>>>>>>> variables or security labels are specified for certain column then in
>>>>>>>>> *dlgTable* class, it was not fetching those variable or security
>>>>>>>>> labels from *dlgColumn *class. So those were not visible on *sql
>>>>>>>>> tab* of new table wizard.
>>>>>>>>>
>>>>>>>>> I have resolved that issue and created the patch for the same.
>>>>>>>>> Patch is attached with this mail. Please review it and if it looks
>>>>>>>>> good, please commit the code.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Sanket Mehta
>>>>>>>>> Sr Software engineer
>>>>>>>>> Enterprisedb
>>>>>>>>>
>>>>>>>>> On Tue, Nov 4, 2014 at 1:16 PM, Sanket Mehta <
>>>>>>>>> sanket(dot)mehta(at)enterprisedb(dot)com> wrote:
>>>>>>>>>
>>>>>>>>>> Sure Ashesh,
>>>>>>>>>>
>>>>>>>>>> I will check and get back.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Regards,
>>>>>>>>>> Sanket Mehta
>>>>>>>>>> Sr Software engineer
>>>>>>>>>> Enterprisedb
>>>>>>>>>>
>>>>>>>>>> On Tue, Nov 4, 2014 at 1:03 PM, Ashesh Vashi <
>>>>>>>>>> ashesh(dot)vashi(at)enterprisedb(dot)com> wrote:
>>>>>>>>>>
>>>>>>>>>>> Sanket,
>>>>>>>>>>>
>>>>>>>>>>> Can you take a look at it?
>>>>>>>>>>>
>>>>>>>>>>> --
>>>>>>>>>>> Thanks,
>>>>>>>>>>>
>>>>>>>>>>> Ashesh Vashi
>>>>>>>>>>>
>>>>>>>>>>> On 4 Nov 2014 12:54, "liuyuanyuan" <liuyuanyuangogo(at)gmail(dot)com>
>>>>>>>>>>> wrote:
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> > Hi , hackers!
>>>>>>>>>>> >
>>>>>>>>>>> > Currently I test some part of pgadmin GUI, and I found some
>>>>>>>>>>> potential invalid input field of New Column GUI,
>>>>>>>>>>> >
>>>>>>>>>>> > Like Variables and Security Labels.
>>>>>>>>>>> Could you please have a look on this issue?
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> > For example:
>>>>>>>>>>> >
>>>>>>>>>>> > OS: WIN7 64bit
>>>>>>>>>>> >
>>>>>>>>>>> > PostgreSQL 9.3
>>>>>>>>>>> >
>>>>>>>>>>> > pgAdmin III:Version 1.18.1
>>>>>>>>>>> >
>>>>>>>>>>> > I use GUI of pgadmin to create table, and add column to this
>>>>>>>>>>> table (just as follow ). When I add a new column,
>>>>>>>>>>> >
>>>>>>>>>>> > I can add Variables (or Security Label )to this column, but
>>>>>>>>>>> finally in the tab SQL of New Table Interface I find nothing
>>>>>>>>>>> >
>>>>>>>>>>> > of the Variables (or Security Label ) I’ve add. It seems that
>>>>>>>>>>> the Variables (or Security Label ) does not work.
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> > Best Wishes!
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>> > Yours
>>>>>>>>>>> >
>>>>>>>>>>> > Jasmine Liu
>>>>>>>>>>> >
>>>>>>>>>>> >
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>>
>>>> --
>>>> *Akshay Joshi*
>>>> *Principal Software Engineer *
>>>>
>>>>
>>>>
>>>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>>>
>>>
>>>
>>
>>
>> --
>> *Akshay Joshi*
>> *Principal Software Engineer *
>>
>>
>>
>> *Phone: +91 20-3058-9517Mobile: +91 976-788-8246*
>>
>
>

--
*Akshay Joshi*
*Principal Software Engineer *

*Phone: +91 20-3058-9517Mobile: +91 976-788-8246*