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

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?

--

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