Re: pgAdmin4 PATCH: Domain Module

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 <
khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:

> Hi Neel,
>
> Please find updated patch.
>
> Thanks,
> Khushboo
>
> On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel(dot)patel(at)enterprisedb(dot)com>
> 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 <
>> khushboo(dot)vashi(at)enterprisedb(dot)com> wrote:
>>
>>> Resending patch with binary option.
>>>
>>> On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi <
>>> khushboo(dot)vashi(at)enterprisedb(dot)com> 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 (pgadmin-hackers(at)postgresql(dot)org)
>>> To make changes to your subscription:
>>> http://www.postgresql.org/mailpref/pgadmin-hackers
>>>
>>>
>>
>