Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.80) (envelope-from ) id 1aLwWY-0002oQ-Ky for pgadmin-hackers@arkaria.postgresql.org; Wed, 20 Jan 2016 17:26:26 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84) (envelope-from ) id 1aLwWY-0001Ir-5F for pgadmin-hackers@arkaria.postgresql.org; Wed, 20 Jan 2016 17:26:26 +0000 Received: from makus.postgresql.org ([2001:4800:1501:1::229]) by malur.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA384:256) (Exim 4.84) (envelope-from ) id 1aLwWJ-0000ms-Q1 for pgadmin-hackers@postgresql.org; Wed, 20 Jan 2016 17:26:12 +0000 Received: from mail-yk0-x22f.google.com ([2607:f8b0:4002:c07::22f]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84) (envelope-from ) id 1aLwWD-0008OS-CB for pgadmin-hackers@postgresql.org; Wed, 20 Jan 2016 17:26:10 +0000 Received: by mail-yk0-x22f.google.com with SMTP id v14so17703168ykd.3 for ; Wed, 20 Jan 2016 09:26:04 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=enterprisedb-com.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type; bh=yrnEHBI8q4OW/2rwO1NBUzUbdKlzHTkF5IsZWvvZdkQ=; b=Y6sVXz623VB3+FgHQLRTW7DZRVM73mTHa57RBiQGbvl/Q6nlqlNZHjdl42kdS+Uu6I a//ktT5hB/GcIm+3q1OMQLxOkpoyTjoCToHT1mSaOlL/m24engeYxYeybL6bZUqcLt5U LcIyV15mGJ8FK/K+1tR2WxeavQf1u2JbTwiAVY0SXnb/L/JluYn6crWhCHtYsuCRY41v 5OYNExOhmmnfGFIQOK6cvCFO7Smd5mA74LXRfTu5s/doEjPa3WBA5RdcNdc1vvbSVIGA it4j2ORIJ2FOcKOI+F755/2x3qzjK0LxLaVTXJ/zKz7QqfpPVlX7tUUwJcz+uV5O/yhb 6x2g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=yrnEHBI8q4OW/2rwO1NBUzUbdKlzHTkF5IsZWvvZdkQ=; b=BiILWXBSrEOlkkUTGNuK5o5mtyrvgx2HlS6xQ8WAl7/PrNL6ARwu3lRkzUdAjZNqbJ rLDJRm6p0OdNPolmHyWE2oPtkweuoe22giPfcgFB29v2EQEWpBd+tTN2mLlECjyyrHNw QkFmcXQmqkTYJ4GEUW7aY8veriqs/n7fSp2QnZxZHAoVPIZAhKE5bh/PQIEetX6UpuzL hncQUwZkisCuSkQ4+ZacRMP6ANmqrcochpstZ6XV9Oqsqqwm/EWM8u8relipNlcGZJau FPGo2Lk2rNwel5zO7VIB3YF5fxqY/KXIzz3ihii4GrnRzBl1l5cYy11E5olvL7AKi6l0 cd5A== X-Gm-Message-State: ALoCoQkWJxnUjEK1esW1hwPauiDYfeTJDMC/1wGu2d9jkFtpHklQxCAngWIX8YxzbPqBS/bPLrxZnUtXLJWF5fi9xHnkqKosTW/IB3SS6JGg2rUJ81sx80M= MIME-Version: 1.0 X-Received: by 10.37.95.84 with SMTP id t81mr11898031ybb.58.1453310764166; Wed, 20 Jan 2016 09:26:04 -0800 (PST) Received: by 10.37.202.75 with HTTP; Wed, 20 Jan 2016 09:26:04 -0800 (PST) In-Reply-To: References: Date: Wed, 20 Jan 2016 22:56:04 +0530 Message-ID: Subject: Re: pgAdmin4 PATCH: Domain Module From: Neel Patel To: Khushboo Vashi Cc: pgadmin-hackers Content-Type: multipart/alternative; boundary=001a11427594d8e14e0529c7478e X-Pg-Spam-Score: -2.6 (--) List-Archive: List-Help: List-ID: List-Owner: List-Post: List-Subscribe: List-Unsubscribe: X-Mailing-List: pgadmin-hackers Precedence: bulk Sender: pgadmin-hackers-owner@postgresql.org --001a11427594d8e14e0529c7478e Content-Type: text/plain; charset=UTF-8 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.vashi@enterprisedb.com> wrote: > Hi Neel, > > Please find updated patch. > > Thanks, > Khushboo > > On Wed, Jan 20, 2016 at 12:50 PM, Neel Patel > 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.vashi@enterprisedb.com> wrote: >> >>> Resending patch with binary option. >>> >>> On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vashi < >>> khushboo.vashi@enterprisedb.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@postgresql.org) >>> To make changes to your subscription: >>> http://www.postgresql.org/mailpref/pgadmin-hackers >>> >>> >> > --001a11427594d8e14e0529c7478e Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable
Hi Khushboo,

Please find below review c= omments.

- While creating new Domain and clicking = on SQL tab, python side we are getting error saying "TypeError: = 9;bool' object is not callable".
=C2=A0 We ar= e not able to create any domain. Fix this issue so that we can test other f= unctionality.
- 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 ne= ed to pass "qtIdent=3Dself.qtIdent" as function argument i= n "create" and "getSQL" function in domains/__init__.py=
- In "Security" tab , provider and security label fiel= ds are not editable.
- In PG version 9.1, when we update the exis= ting domain name then "ALTER DOMAIN" is not supported.=C2=A0
=C2=A0 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

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

- Some of t= he SQL file, qtIdent is not used. Please check all the related SQL files.
=C2=A0 e.g. =C2=A0- In update.sql file "data.owner" shou= ld be "conn|qtIdent(data.owner)"

=C2=A0 = =C2=A0 {% if data.owner %}
=C2=A0 =C2=A0 ALTER DOMAIN {{ conn|qtI= dent(o_data.basensp, name) }}
=C2=A0 =C2=A0 =C2=A0 OWNER TO {{ da= ta.owner }};
=C2=A0 {% endif %}

Let us k= now for any issues.

Thanks,
Neel Patel

On Wed,= Jan 20, 2016 at 2:50 PM, Khushboo Vashi <khushboo.vashi@ent= erprisedb.com> wrote:
Hi Neel,

Please find updated patch.=

Thanks,
Khushboo

On = Wed, Jan 20, 2016 at 12:50 PM, Neel Patel <neel.patel@enterprise= db.com> wrote:
Hi Khushboo,

While applying the patch file, we are = getting below warnings.

##########################= ###############
domains (1).patch:1340: trailing whitespace.=
=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 oid: undefined,=C2=A0
domains (1).patch:1483: trailing whitespace.
=C2=A0 =C2=A0 (ns= pname =3D 'pg_catalog' AND EXISTS=C2=A0
domains (1).patch= :1487: trailing whitespace.
=C2=A0 =C2=A0 OR (nspname =3D 'in= formation_schema' AND EXISTS=C2=A0
domains (1).patch:1489: tr= ailing whitespace.
=C2=A0 =C2=A0 OR (nspname LIKE '_%' AN= D EXISTS=C2=A0
domains (1).patch:1642: trailing whitespace.
=
=C2=A0(select 1 from pg_class where relnamespace=3Dtypnamespace and re= lname =3D typname and relkind !=3D 'c') AND (typname not like '= _%' OR NOT EXISTS (select 1 from pg_class where relnamespace=3Dtypnames= pace and relname =3D substring(typname from 2)::name and relkind !=3D '= c'))=C2=A0
warning: squelched 4 whitespace errors
w= arning: 9 lines add whitespace errors.
####################= #####################

Can you please remove th= e whitespace and regenerate the patch ?

Thanks,
Neel Patel

On Wed, Jan 20, 2016 at 12:37 PM, Khushboo Vashi <khushboo.vashi@enterprisedb.com> wrote:
Resending = patch with binary option.
On Wed, Jan 20, 2016 at 10:18 AM, Khushboo Vash= i <khushboo.vashi@enterprisedb.com> wrote:
=
Hi,
=
Please find attached patch for the Domain Module.

Th= e patch will be modified after Types module implementation as we need to po= pulate Base Type=C2=A0 and some Type related validations from the Types mod= ule.

Please review it and let me know the feedback.

Thanks,
Khushboo



--
Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers=




--001a11427594d8e14e0529c7478e--