public inbox for [email protected]  
help / color / mirror / Atom feed
From: Neel Patel <[email protected]>
To: Khushboo Vashi <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: pgAdmin4 PATCH: Domain Module
Date: Wed, 20 Jan 2016 22:56:04 +0530
Message-ID: <CACCA4P06PrJ2-VodT133sheH1i+7zc_M_y_me6KfRHkLqrQ7gg@mail.gmail.com> (raw)
In-Reply-To: <CAFOhELdgb566u+4XXOOP4pDCF4GGzK8JZzLrrTuPt=OprsqG5Q@mail.gmail.com>
References: <CAFOhELf-n8mM4h8RZuqxUs-Z+f97N6Ux5KT6aoAdWFiHVVURyw@mail.gmail.com>
	<CAFOhELf-QTMAgNkTT8AsRwJ38Jn_1BSFRV_G5ZmcHnrKmPsTog@mail.gmail.com>
	<CACCA4P1caumXOrpEXrrETMuNGj3G3ctY-PN9=V6GUPF=dTBnWg@mail.gmail.com>
	<CAFOhELdgb566u+4XXOOP4pDCF4GGzK8JZzLrrTuPt=OprsqG5Q@mail.gmail.com>
List-Unsubscribe:  <mailto:[email protected]?body=unsub%20pgadmin-hackers>

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


view thread (29+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected]
  Subject: Re: pgAdmin4 PATCH: Domain Module
  In-Reply-To: <CACCA4P06PrJ2-VodT133sheH1i+7zc_M_y_me6KfRHkLqrQ7gg@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox