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.
3. dlgColumn.cpp
- Remove comment //code removed for testing from constructor if
testing is done.
On Tue, Nov 18, 2014 at 6:10 PM, Sanket Mehta <sanket.me...@enterprisedb.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.me...@enterprisedb.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.va...@enterprisedb.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.va...@enterprisedb.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.me...@enterprisedb.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.me...@enterprisedb.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.va...@enterprisedb.com> wrote:
>>>>>>
>>>>>>> Sanket,
>>>>>>>
>>>>>>> Can you take a look at it?
>>>>>>>
>>>>>>> --
>>>>>>> Thanks,
>>>>>>>
>>>>>>> Ashesh Vashi
>>>>>>>
>>>>>>> On 4 Nov 2014 12:54, "liuyuanyuan" <liuyuanyuang...@gmail.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*
