Hi Khushboo,

Please find below review comments.

- While creating new Domain and clicking on SQL tab, python side we are
getting error saying "*TypeError: 'bool' object is not callable".*
  We are not able to create any domain. Fix this issue so that we can test
other functionality.
- Implement the reverse engineering SQL generation for the domain node.
- As per the checklist, remove the "Use Slony" from Constraints tab, as it
is not required.
- No need to pass "*qtIdent=self.qtIdent*" as function argument in "create"
and "getSQL" function in domains/__init__.py
- In "Security" tab , provider and security label fields are not editable.
- In PG version 9.1, when we update the existing domain name then "ALTER
DOMAIN" is not supported.
  Currently there is no checking for the PG version 9.1 and 9.2_plus. It
will fail when we connect to database 9.1

  e.g.
  For PG version 9.1 - Update command should be as below.
  ALTER TYPE xyz RENAME TO abc;
  For PG version 9.2 onwards - Update command should be as below.
  ALTER DOMAIN xyz RENAME TO abc;

- Some of the SQL file, qtIdent is not used. Please check all the related
SQL files.
  e.g.  - In update.sql file "data.owner" should be
"conn|qtIdent(data.owner)"

    {% if data.owner %}
    ALTER DOMAIN {{ conn|qtIdent(o_data.basensp, name) }}
      OWNER TO {{ data.owner }};
  {% endif %}

Let us know for any issues.

Thanks,
Neel Patel

On Wed, Jan 20, 2016 at 2:50 PM, Khushboo Vashi <
[email protected]> wrote:

> Hi Neel,
>
> Please find updated patch.
>
> Thanks,
> Khushboo
>
> On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <[email protected]>
> wrote:
>
>> Hi Khushboo,
>>
>> While applying the patch file, we are getting below warnings.
>>
>> #########################################
>> domains (1).patch:1340: trailing whitespace.
>>           oid: undefined,
>> domains (1).patch:1483: trailing whitespace.
>>     (nspname = 'pg_catalog' AND EXISTS
>> domains (1).patch:1487: trailing whitespace.
>>     OR (nspname = 'information_schema' AND EXISTS
>> domains (1).patch:1489: trailing whitespace.
>>     OR (nspname LIKE '_%' AND EXISTS
>> domains (1).patch:1642: trailing whitespace.
>>  (select 1 from pg_class where relnamespace=typnamespace and relname =
>> typname and relkind != 'c') AND (typname not like '_%' OR NOT EXISTS
>> (select 1 from pg_class where relnamespace=typnamespace and relname =
>> substring(typname from 2)::name and relkind != 'c'))
>> warning: squelched 4 whitespace errors
>> warning: 9 lines add whitespace errors.
>> #########################################
>>
>> Can you please remove the whitespace and regenerate the patch ?
>>
>> Thanks,
>> Neel Patel
>>
>> On Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <
>> [email protected]> wrote:
>>
>>> Resending patch with binary option.
>>>
>>> On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <
>>> [email protected]> wrote:
>>>
>>>> Hi,
>>>>
>>>> Please find attached patch for the Domain Module.
>>>>
>>>> The patch will be modified after Types module implementation as we need
>>>> to populate Base Type  and some Type related validations from the Types
>>>> module.
>>>>
>>>> Please review it and let me know the feedback.
>>>>
>>>> Thanks,
>>>> Khushboo
>>>>
>>>
>>>
>>>
>>> --
>>> Sent via pgadmin-hackers mailing list ([email protected])
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>
>

Reply via email to