Received: from malur.postgresql.org ([217.196.149.56]) by arkaria.postgresql.org with esmtp (Exim 4.84_2) (envelope-from ) id 1cRWwd-0003Fj-Nm for pgadmin-hackers@arkaria.postgresql.org; Thu, 12 Jan 2017 04:24:59 +0000 Received: from localhost ([127.0.0.1] helo=postgresql.org) by malur.postgresql.org with smtp (Exim 4.84_2) (envelope-from ) id 1cRWwb-0003dy-Vq for pgadmin-hackers@arkaria.postgresql.org; Thu, 12 Jan 2017 04:24:58 +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_2) (envelope-from ) id 1cRWwa-0003dp-4c for pgadmin-hackers@postgresql.org; Thu, 12 Jan 2017 04:24:56 +0000 Received: from mail-yb0-x229.google.com ([2607:f8b0:4002:c09::229]) by makus.postgresql.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_CBC_SHA1:256) (Exim 4.84_2) (envelope-from ) id 1cRWwR-0001gI-86 for pgadmin-hackers@postgresql.org; Thu, 12 Jan 2017 04:24:53 +0000 Received: by mail-yb0-x229.google.com with SMTP id v132so2489253yba.0 for ; Wed, 11 Jan 2017 20:24:45 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=pivotal-io.20150623.gappssmtp.com; s=20150623; h=mime-version:in-reply-to:references:from:date:message-id:subject:to :cc; bh=NwMzvEO4zXdE9x4ApKsYMntngOV3Mj6FnRHLdu+WX0g=; b=uQpk3R3dnM9yCxXcAocGeD0ccbAFdL8oyh834qNp2xNylooVOgk0XFeVc3KvCRAwfs cKc8NNxuJxfGmQmH5OKpw4rFvK2arswhCzQxDHH+UHWI9K5EYlgpajL8EE56h9B63K/4 usQVGelej5OMA5Uu4WHikcEIp4b4/p2rkjjT9XKj1tZqjXVD+ajuUXwIhOmAyV/MwoIy OmaWXspwnF4fiurlZZ/GMoEcSl5eLN++J7gZgDj+QOMYsOV/DnbZDpCAwdUcmjkeZLN1 f5bxcw0spm6NortudidPp7tH1X+r62MFYzFj/fahhNqSZrH2TM2cEDgGjrPbq8zmK+lA 0Rpw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=NwMzvEO4zXdE9x4ApKsYMntngOV3Mj6FnRHLdu+WX0g=; b=ZVwlzb62Pmw4StQTIQduhpdB9xSiJFD33vsoYtmLk4qDK1tlnSir0DBp2ds81hX3YL w5pYfTYjSvidHcm4CFc8Jll8nd0NcWKw8RK/wdnJPpTYJHBbvOpF5XnWR2aGwzIvB0kW exYVkeGNAgcd9ILX0ro8U9xB3fR7BO+WBelHdfjMc89EpVp5LSqFbSmzjKyXlcfqv5gU G3R64schsAn/YPGMrDEVfEwykECypgvlwJCYVIQBnCfb7VTsmtsQzFj7Nu2p2UYu6EvX QYFEQsmJsqJvO4V6nQTcQlYw9ccEOeiTJijTM3vlNCeLwZg1+5lZvpiL3RAOEz/Azcyk suxQ== X-Gm-Message-State: AIkVDXLWedKX9LgXNIs9eftWXVZonD7WTQt50jIYowFVdXc0tkkT0hdy6A2tJ96I1Sd5nSbfSC9Ay1HmgBUhgUb+ X-Received: by 10.37.174.9 with SMTP id a9mr6689259ybj.153.1484195085417; Wed, 11 Jan 2017 20:24:45 -0800 (PST) MIME-Version: 1.0 Received: by 10.83.9.140 with HTTP; Wed, 11 Jan 2017 20:24:44 -0800 (PST) In-Reply-To: References: From: Robert Eckhardt Date: Wed, 11 Jan 2017 20:24:44 -0800 Message-ID: Subject: Re: Driver Module To: Dave Page Cc: George Gelashvili , "pgadmin-hackers@postgresql.org" , "plumadmin@pivotal.io" , Ashesh Vashi Content-Type: multipart/alternative; boundary=f403045da354d82eb70545de18d7 X-Pg-Spam-Score: -1.9 (-) 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 --f403045da354d82eb70545de18d7 Content-Type: text/plain; charset=UTF-8 On Wed, Jan 11, 2017 at 8:07 PM, Dave Page wrote: > Hi > > On Wed, Jan 11, 2017 at 10:24 PM, George Gelashvili > wrote: > > Hi Dave, > > > > Thanks for the pointer. > > We realized that many of the changes we would need to make for supporting > > Greenplum would need to go where there is pg version checking throughout > the > > code. This is because unlike PPAS which mostly adds additional features, > > Greenplum is based on postgres 8.3. > > Isn't Heikki fixing that for your next release? > The current release is 8.2, we aren't trying to make that work with pgAdmin4. Heikki did a yeomans work and the next release will be based on 8.3. Future releases should be more than one Postgres version at a time but there was a lot of cleanup to do before we could start the Postgres merging. > > > It looks like much of the version checking logic is repeated at points > where > > the features are differentiated by postgres version. > > > > It might make sense at this point to refactor the way that feature > flagging > > is done to be a little bit more unified between server types and postgres > > versions so that we could for example have logic along the lines of: > > > > feature_enablement = FeatureEnablement(postgres_flavor, > postgres_version) > > > > #... > > > > if(feature_enablement.check_internal_triggers ): > > # feature call here > > > > and then in a feature enablement class, reference the various versions > and > > flavors of postgres. > > > > Any thoughts on this? > > I worry that the list of features would end up being huge - we're not > just talking about basic things like whether DDL triggers are > supported, but the catalog schema (e.g. procpid vs. pid in > pg_stat_activity) and small things like whether a particular GUC can > be set on a tablespace. > > Ultimately, you have to do a version check at some point though > (unless you're proposing to do something similar to probing the DOM in > a browser at runtime). Doesn't GP's version string contain additional > info beyond '8.3'? In pgAdmin 3 we had a EdbMinimumVersion(int major, > int minor) function in the connection class that basically did: > > return isEdb && BackendMinimumVersion(x, y); > > Something like that could check other elements of the GP version number. > Greenplum is about to start leveraging semantic versioning. The version number for the next release will be 5.0.0. > > -- > Dave Page > Blog: http://pgsnake.blogspot.com > Twitter: @pgsnake > > EnterpriseDB UK: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgadmin-hackers > --f403045da354d82eb70545de18d7 Content-Type: text/html; charset=UTF-8 Content-Transfer-Encoding: quoted-printable


On Wed, Jan 11, 2017 at 8:07 PM, Dave Page <dpage@pgadmin.org><= /span> wrote:
Hi

On Wed, Jan 11, 2017 at 10:24 PM, George Gelashvili
<ggelashvili@pivotal.io>= ; wrote:
> Hi Dave,
>
> Thanks for the pointer.
> We realized that many of the changes we would need to make for support= ing
> Greenplum would need to go where there is pg version checking througho= ut the
> code. This is because unlike PPAS which mostly adds additional feature= s,
> Greenplum is based on postgres 8.3.

Isn't Heikki fixing that for your next release?
=

The current release is 8.2, we aren't trying to mak= e that work with pgAdmin4. Heikki did a yeomans work and the next release w= ill be based on 8.3. Future releases should be more than one Postgres versi= on at a time but there was a lot of cleanup to do before we could start the= Postgres merging.=C2=A0
=C2=A0

> It looks like much of the version checking logic is repeated at points= where
> the features are differentiated by postgres version.
>
> It might make sense at this point to refactor the way that feature fla= gging
> is done to be a little bit more unified between server types and postg= res
> versions so that we could for example have logic along the lines of: >
> feature_enablement =3D FeatureEnablement(postgres_flavor, postgre= s_version)
>
> #...
>
> if(feature_enablement.check_internal_triggers ):
>=C2=A0 =C2=A0# feature call here
>
> and then in a feature enablement class, reference the various versions= and
> flavors of postgres.
>
> Any thoughts on this?

I worry that the list of features would end up being huge - we'r= e not
just talking about basic things like whether DDL triggers are
supported, but the catalog schema (e.g. procpid vs. pid in
pg_stat_activity) and small things like whether a particular GUC can
be set on a tablespace.

Ultimately, you have to do a version check at some point though
(unless you're proposing to do something similar to probing the DOM in<= br> a browser at runtime). Doesn't GP's version string contain addition= al
info beyond '8.3'? In pgAdmin 3 we had a EdbMinimumVersion(int majo= r,
int minor) function in the connection class that basically did:

return isEdb && BackendMinimumVersion(x, y);

Something like that could check other elements of the GP version number.

Greenplum is about to start leveraging se= mantic versioning. The version number for the next release will be 5.0.0.= =C2=A0
=C2=A0

--
Dave Page
Blog: http://pgsnake.blogspot.com
Twitter: @pgsnake

EnterpriseDB UK: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


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

--f403045da354d82eb70545de18d7--