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

Hi Ashesh,

On Wed, Nov 26, 2014 at 7:35 PM, Ashesh Vashi <ashesh(dot)vashi(at)enterprisedb(dot)com
> wrote:

> Hi Sanket,
>
> On Wed, Nov 26, 2014 at 5:41 PM, Akshay Joshi <
> akshay(dot)joshi(at)enterprisedb(dot)com> wrote:
>
>> 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.
>>>
>> I can see the memory leak, while closing the table dialog.
> While changing the column, we do create column2 for preserving the data,
> but it is never been released.
>
> Can you please cross check?
>
> *Resolved.*
>
*PFA the revised patch.*

>
>
--
>
> 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 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*
>>
>
>