public inbox for [email protected]
help / color / mirror / Atom feedFrom: Surinder Kumar <[email protected]>
To: Dave Page <[email protected]>
Cc: pgadmin-hackers <[email protected]>
Subject: Re: [pgAdmin4][Patch]: RM1840 - cannot create gist index due to enforced ASC, DESC options in generated SQL
Date: Fri, 13 Jan 2017 12:20:46 +0530
Message-ID: <CAM5-9D-+uETwn4_LqHOGAwk46Wg6U-udSV_WrAy=FHnKwffhFA@mail.gmail.com> (raw)
In-Reply-To: <CA+OCxoyCJuY-Edh7K8uX8v9f+uYAS0YpQMmnzqPn5AZyY9WgKw@mail.gmail.com>
References: <CAM5-9D-ti4O0bE3W_=puMMWPjO4qR525gYfKuGSYAiGow-AZdQ@mail.gmail.com>
<CA+OCxowGDMCf_YJauW=u24X_7X4XMnT5tkca4qMRZCpH8z2mDA@mail.gmail.com>
<CAM5-9D8XX_BVPi2XQODAK5DPRrhi9sTMHQDh_KTu0p9FjkU=dQ@mail.gmail.com>
<CA+OCxozBD1OBWQTG5SvdnVFipuScgK1nNaXUxazDSXqif_gtzQ@mail.gmail.com>
<CAM5-9D8osVRFd4s9kps-Bnuqw53WPFT4TN8ayuOO0Hr5nzuwxA@mail.gmail.com>
<CA+OCxowo2158=USKZ1nFvpogA4BO2R3UcHHC6xWzSsiGOdj6eg@mail.gmail.com>
<CAM5-9D97YhGDXQZfOg+FBf0nDedQvydWH+mAmr2iRi0rOL9PSA@mail.gmail.com>
<CA+OCxoyCJuY-Edh7K8uX8v9f+uYAS0YpQMmnzqPn5AZyY9WgKw@mail.gmail.com>
List-Unsubscribe: <mailto:[email protected]?body=unsub%20pgadmin-hackers>
Hi
Please find updated patch following changes:
1) Keep field 'opclass' combo box enabled.
2) Keep ASC/DESC and NULLs FIRST/LAST options disable for access methods
other than 'btree'.
3) Add validation for name field.
On Fri, Nov 25, 2016 at 5:16 PM, Dave Page <[email protected]> wrote:
> On Thu, Nov 24, 2016 at 12:13 PM, Surinder Kumar
> <[email protected]> wrote:
> > Hi
> >
> > Please find updated patch with change.
> >
> > On Fri, Oct 21, 2016 at 9:16 PM, Dave Page <[email protected]> wrote:
> >>
> >> On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
> >> <[email protected]> wrote:
> >> > On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <[email protected]> wrote:
> >> >>
> >> >> Hi
> >> >>
> >> >> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
> >> >> <[email protected]> wrote:
> >> >> > Hi
> >> >> >
> >> >> > This fix is for exclusion constraint.
> >> >> > The options like "order" and "nulls" must be conditional. i.e.
> >> >> > include
> >> >> > only
> >> >> > when access method type is other than "gist".
> >> >>
> >> >> When creating an index, the asc/desc options are disabled if gist/gin
> >> >> used. I think they also should be here.
> >> >>
> >> >> Also, what about gin indexes in this case?
> >> >
> >> > As per documentation,
> >> > The access method must support amgettuple (see Chapter 52); at present
> >> > this
> >> > means GIN cannot be used
> >>
> >> OK, but this patch (unlike the last one) only seems to check for gist.
> >
> > I have modified the code so It will check for 'gist' and 'spgist'
>
> Hi,
>
> This still doesn't seem right to me. For example, if I choose an
> access method that doesn't have a default operator class for the
> selected data type, Postgres asks me to explicitly choose one, which I
> now can't because the combo box is disabled. Conversely, whilst the
> opclass should probably not be disabled, the ASC/DESC and NULLs
> FIRST/LAST options probably should be disabled (right now, they're
> just ignored).
>
> Thoughts?
>
> --
> 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 ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers
Attachments:
[application/octet-stream] RM1840_V1.patch (9.4K, 3-RM1840_V1.patch)
download | inline diff:
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/__init__.py b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/__init__.py
index d099502..3319b7f 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/__init__.py
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/__init__.py
@@ -816,7 +816,7 @@ class ExclusionConstraintView(PGChildNodeView):
sql = render_template("/".join([self.template_path, 'create.sql']),
data=data, conn=self.conn)
- return sql, data['name'] if 'name' in data else old_data['name']
+ return sql, data['name']
@check_precondition
def sql(self, gid, sid, did, scid, tid, exid=None):
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js
index 9250d9f..42b4336 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/constraints/exclusion_constraint/templates/exclusion_constraint/js/exclusion_constraint.js
@@ -23,26 +23,9 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) {
},{
id: 'oper_class', label:'{{ _('Operator class') }}', type:'text',
node: 'table', url: 'get_oper_class', first_empty: true,
- editable: function(m) {
- if (m instanceof Backbone.Collection) {
- return true;
- }
- if ((_.has(m.collection, 'handler') &&
- !_.isUndefined(m.collection.handler) &&
- !_.isUndefined(m.collection.handler.get('oid')))) {
- return false;
- }
-
- if (m.collection) {
- var indexType = m.collection.handler.get('amname')
- return (indexType == 'btree' || _.isUndefined(indexType) ||
- _.isNull(indexType) || indexType == '');
- } else {
- return true;
- }
- },
+ editable: true,
select2: {
- allowClear: true, width: 'style',
+ allowClear: true, width: 'style', tags: true,
placeholder: '{{ _("Select the operator class") }}'
}, cell: Backgrid.Extension.Select2Cell.extend({
initialize: function () {
@@ -105,7 +88,10 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) {
onText: 'ASC',
offText: 'DESC',
},editable: function(m) {
- if (m instanceof Backbone.Collection) {
+ if (_.contains(['gist', 'spgist', 'hash'], m.top.get('amname'))) {
+ return false;
+ }
+ else if (m instanceof Backbone.Collection) {
return true;
}
if ((_.has(m.collection, 'handler') &&
@@ -121,7 +107,10 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) {
onText: 'FIRST',
offText: 'LAST',
},editable: function(m) {
- if (m instanceof Backbone.Collection) {
+ if (_.contains(['gist', 'spgist', 'hash'], m.top.get('amname'))) {
+ return false;
+ }
+ else if (m instanceof Backbone.Collection) {
return true;
}
if ((_.has(m.collection, 'handler') &&
@@ -894,8 +883,15 @@ function($, _, S, pgAdmin, pgBrowser, Alertify) {
}],
validate: function() {
this.errorModel.clear();
- var columns = this.get('columns');
- if ((_.isUndefined(columns) || _.isNull(columns) || columns.length < 1)) {
+ var columns = this.get('columns'),
+ name = this.get('name');
+
+ if ((_.isUndefined(name) || _.isNull(name) || name.length < 1)) {
+ var msg = '{{ _('Please specify name for exclusion constraint.') }}';
+ this.errorModel.set('name', msg);
+ return msg;
+ }
+ else if ((_.isUndefined(columns) || _.isNull(columns) || columns.length < 1)) {
var msg = '{{ _('Please specify columns for exclusion constraint.') }}';
this.errorModel.set('columns', msg);
return msg;
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.1_plus/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.1_plus/create.sql
index db29048..80c427a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.1_plus/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.1_plus/create.sql
@@ -1,7 +1,7 @@
ALTER TABLE {{ conn|qtIdent(data.schema, data.table) }}
ADD{% if data.name %} CONSTRAINT {{ conn|qtIdent(data.name) }}{% endif%} EXCLUDE {% if data.amname and data.amname != '' %}USING {{data.amname}}{% endif %} (
{% for col in data.columns %}{% if loop.index != 1 %},
- {% endif %}{{ conn|qtIdent(col.column)}} {% if col.oper_class and col.oper_class != '' %}{{col.oper_class}} {% endif%}{% if col.order %}ASC{% else %}DESC{% endif %} NULLS {% if col.nulls_order %}FIRST{% else %}LAST{% endif %} WITH {{col.operator}}{% endfor %}){% if data.fillfactor %}
+ {% endif %}{{ conn|qtIdent(col.column)}}{% if col.oper_class and col.oper_class != '' %} {{col.oper_class}}{% endif%}{% if col.order is defined %}{% if col.order %} ASC{% else %} DESC{% endif %} NULLS{% endif %} {% if col.nulls_order is defined %}{% if col.nulls_order %}FIRST {% else %}LAST {% endif %}{% endif %}WITH {{col.operator}}{% endfor %}){% if data.fillfactor %}
WITH (FILLFACTOR={{data.fillfactor}}){% endif %}{% if data.spcname and data.spcname != "pg_default" %}
USING INDEX TABLESPACE {{ conn|qtIdent(data.spcname) }}{% endif %}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.2_plus/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.2_plus/create.sql
index db29048..80c427a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.2_plus/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.2_plus/create.sql
@@ -1,7 +1,7 @@
ALTER TABLE {{ conn|qtIdent(data.schema, data.table) }}
ADD{% if data.name %} CONSTRAINT {{ conn|qtIdent(data.name) }}{% endif%} EXCLUDE {% if data.amname and data.amname != '' %}USING {{data.amname}}{% endif %} (
{% for col in data.columns %}{% if loop.index != 1 %},
- {% endif %}{{ conn|qtIdent(col.column)}} {% if col.oper_class and col.oper_class != '' %}{{col.oper_class}} {% endif%}{% if col.order %}ASC{% else %}DESC{% endif %} NULLS {% if col.nulls_order %}FIRST{% else %}LAST{% endif %} WITH {{col.operator}}{% endfor %}){% if data.fillfactor %}
+ {% endif %}{{ conn|qtIdent(col.column)}}{% if col.oper_class and col.oper_class != '' %} {{col.oper_class}}{% endif%}{% if col.order is defined %}{% if col.order %} ASC{% else %} DESC{% endif %} NULLS{% endif %} {% if col.nulls_order is defined %}{% if col.nulls_order %}FIRST {% else %}LAST {% endif %}{% endif %}WITH {{col.operator}}{% endfor %}){% if data.fillfactor %}
WITH (FILLFACTOR={{data.fillfactor}}){% endif %}{% if data.spcname and data.spcname != "pg_default" %}
USING INDEX TABLESPACE {{ conn|qtIdent(data.spcname) }}{% endif %}
diff --git a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.6_plus/create.sql b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.6_plus/create.sql
index db29048..80c427a 100644
--- a/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.6_plus/create.sql
+++ b/web/pgadmin/browser/server_groups/servers/databases/schemas/tables/templates/exclusion_constraint/sql/9.6_plus/create.sql
@@ -1,7 +1,7 @@
ALTER TABLE {{ conn|qtIdent(data.schema, data.table) }}
ADD{% if data.name %} CONSTRAINT {{ conn|qtIdent(data.name) }}{% endif%} EXCLUDE {% if data.amname and data.amname != '' %}USING {{data.amname}}{% endif %} (
{% for col in data.columns %}{% if loop.index != 1 %},
- {% endif %}{{ conn|qtIdent(col.column)}} {% if col.oper_class and col.oper_class != '' %}{{col.oper_class}} {% endif%}{% if col.order %}ASC{% else %}DESC{% endif %} NULLS {% if col.nulls_order %}FIRST{% else %}LAST{% endif %} WITH {{col.operator}}{% endfor %}){% if data.fillfactor %}
+ {% endif %}{{ conn|qtIdent(col.column)}}{% if col.oper_class and col.oper_class != '' %} {{col.oper_class}}{% endif%}{% if col.order is defined %}{% if col.order %} ASC{% else %} DESC{% endif %} NULLS{% endif %} {% if col.nulls_order is defined %}{% if col.nulls_order %}FIRST {% else %}LAST {% endif %}{% endif %}WITH {{col.operator}}{% endfor %}){% if data.fillfactor %}
WITH (FILLFACTOR={{data.fillfactor}}){% endif %}{% if data.spcname and data.spcname != "pg_default" %}
USING INDEX TABLESPACE {{ conn|qtIdent(data.spcname) }}{% endif %}
view thread (14+ 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]: RM1840 - cannot create gist index due to enforced ASC, DESC options in generated SQL
In-Reply-To: <CAM5-9D-+uETwn4_LqHOGAwk46Wg6U-udSV_WrAy=FHnKwffhFA@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