Thread: [pgAdmin4][Patch]: RM1840 - cannot create gist index due to enforced ASC, DESC options in generated SQL

Hi,

The options like "sort" and "nulls" must be conditional. i.e. include only when access method type is other than "gist" or "gin".

Please find attached patch and review.

Thanks,
Surinder Kumar

Attachment
Thanks, applied.

On Thu, Oct 20, 2016 at 7:47 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi,
>
> The options like "sort" and "nulls" must be conditional. i.e. include only
> when access method type is other than "gist" or "gin".
>
> Please find attached patch and review.
>
> Thanks,
> Surinder Kumar
>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



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

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


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

Please find attached patch and review.

On Fri, Oct 21, 2016 at 4:38 PM, Dave Page <dpage@pgadmin.org> wrote:
Thanks, applied.

On Thu, Oct 20, 2016 at 7:47 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi,
>
> The options like "sort" and "nulls" must be conditional. i.e. include only
> when access method type is other than "gist" or "gin".
>
> Please find attached patch and review.
>
> Thanks,
> Surinder Kumar
>
>
>
> --
> Sent via pgadmin-hackers mailing list (pgadmin-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgadmin-hackers
>



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

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

Attachment
Hi

On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> 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?

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

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


On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> 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

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

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

On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> 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.

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

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


Hi

Please find updated patch with change.

On Fri, Oct 21, 2016 at 9:16 PM, Dave Page <dpage@pgadmin.org> wrote:
On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> 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'

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

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

Attachment
On Thu, Nov 24, 2016 at 12:13 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> Please find updated patch with change.
>
> On Fri, Oct 21, 2016 at 9:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.com> 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


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 <dpage@pgadmin.org> wrote:
On Thu, Nov 24, 2016 at 12:13 PM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> Please find updated patch with change.
>
> On Fri, Oct 21, 2016 at 9:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.com> 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

Attachment
Hi

On Fri, Jan 13, 2017 at 6:50 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> Please find updated patch following changes:
> 1) Keep field 'opclass' combo box enabled.

That doesn't seem to be working.

> 2) Keep ASC/DESC and NULLs FIRST/LAST options disable for access methods
> other than 'btree'.
> 3) Add validation for name field.

Those bits do though :-)

Please fix 1).

Thanks!

> On Fri, Nov 25, 2016 at 5:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Thu, Nov 24, 2016 at 12:13 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> >
>> > Please find updated patch with change.
>> >
>> > On Fri, Oct 21, 2016 at 9:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.com> wrote:
>> >> > On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >> >>
>> >> >> Hi
>> >> >>
>> >> >> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
>> >> >> <surinder.kumar@enterprisedb.com> 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
>
>



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

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


Hi
On Tue, Jan 17, 2017 at 3:08 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Fri, Jan 13, 2017 at 6:50 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
>
> Please find updated patch following changes:
> 1) Keep field 'opclass' combo box enabled.

That doesn't seem to be working.
​Is the field 'opclass' is not enabled for you?.
Can you please tell in which scenario it doesn't gets enabled? so that I can fix it.​

> 2) Keep ASC/DESC and NULLs FIRST/LAST options disable for access methods
> other than 'btree'.
> 3) Add validation for name field.

Those bits do though :-)

Please fix 1).

Thanks!

> On Fri, Nov 25, 2016 at 5:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> On Thu, Nov 24, 2016 at 12:13 PM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> >
>> > Please find updated patch with change.
>> >
>> > On Fri, Oct 21, 2016 at 9:16 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> On Fri, Oct 21, 2016 at 4:42 PM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.com> wrote:
>> >> > On Fri, Oct 21, 2016 at 8:52 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >> >>
>> >> >> Hi
>> >> >>
>> >> >> On Fri, Oct 21, 2016 at 4:16 PM, Surinder Kumar
>> >> >> <surinder.kumar@enterprisedb.com> 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
>
>



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

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

Hi

On Tue, Jan 17, 2017 at 9:52 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
> On Tue, Jan 17, 2017 at 3:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Fri, Jan 13, 2017 at 6:50 AM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> >
>> > Please find updated patch following changes:
>> > 1) Keep field 'opclass' combo box enabled.
>>
>> That doesn't seem to be working.
>
> Is the field 'opclass' is not enabled for you?.

No.

> Can you please tell in which scenario it doesn't gets enabled? so that I can
> fix it.

PostgreSQL 9.4, table:

CREATE TABLE public.foo
(
    id text COLLATE pg_catalog."default" NOT NULL,
    data1 text COLLATE pg_catalog."default",
    CONSTRAINT foo_pkey1 PRIMARY KEY (id),
    CONSTRAINT gerp1 UNIQUE (id, data1)
)

1) Right-click, Create index

2) Name: xxxx

3) Access method: gist

4) Add column. Select id (note the opclass field is not enabled)

5) Add column. Select data1 (note the opclass field is not enabled).

6) Click save, and behold the following error in all it's glory :-)

ERROR: data type text has no default operator class for access method "gist"
HINT: You must specify an operator class for the index or define a
default operator class for the data type.

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

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


Hi Dave,

The last patch was for exclusion constraint only. The same issue issue is reported RM2061 for index. 
So, this patch takes care of both issues.

Added a flag 'is_sort_nulls_applicable' which is set to true for access method 'btree'.
If its is true, the values for options 'ASC/DESC' and 'NULLS' are included in 'create' sql for exclusion and index otherwise not.

Please find updated patch and review.

On Tue, Jan 17, 2017 at 3:43 PM, Dave Page <dpage@pgadmin.org> wrote:
Hi

On Tue, Jan 17, 2017 at 9:52 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi
> On Tue, Jan 17, 2017 at 3:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Fri, Jan 13, 2017 at 6:50 AM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> >
>> > Please find updated patch following changes:
>> > 1) Keep field 'opclass' combo box enabled.
>>
>> That doesn't seem to be working.
​It is now fixed in the updated patch.​
>
> Is the field 'opclass' is not enabled for you?.

No.

> Can you please tell in which scenario it doesn't gets enabled? so that I can
> fix it.

PostgreSQL 9.4, table:

CREATE TABLE public.foo
(
    id text COLLATE pg_catalog."default" NOT NULL,
    data1 text COLLATE pg_catalog."default",
    CONSTRAINT foo_pkey1 PRIMARY KEY (id),
    CONSTRAINT gerp1 UNIQUE (id, data1)
)

1) Right-click, Create index

2) Name: xxxx

3) Access method: gist

4) Add column. Select id (note the opclass field is not enabled)

5) Add column. Select data1 (note the opclass field is not enabled).

6) Click save, and behold the following error in all it's glory :-)

ERROR: data type text has no default operator class for access method "gist"
HINT: You must specify an operator class for the index or define a
default operator class for the data type.​
​Thanks for steps.​

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

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

Attachment
Thanks, patch applied.

On Fri, Jan 20, 2017 at 7:29 AM, Surinder Kumar
<surinder.kumar@enterprisedb.com> wrote:
> Hi Dave,
>
> The last patch was for exclusion constraint only. The same issue issue is
> reported RM2061 for index.
> So, this patch takes care of both issues.
>
> Added a flag 'is_sort_nulls_applicable' which is set to true for access
> method 'btree'.
> If its is true, the values for options 'ASC/DESC' and 'NULLS' are included
> in 'create' sql for exclusion and index otherwise not.
>
> Please find updated patch and review.
>
> On Tue, Jan 17, 2017 at 3:43 PM, Dave Page <dpage@pgadmin.org> wrote:
>>
>> Hi
>>
>> On Tue, Jan 17, 2017 at 9:52 AM, Surinder Kumar
>> <surinder.kumar@enterprisedb.com> wrote:
>> > Hi
>> > On Tue, Jan 17, 2017 at 3:08 PM, Dave Page <dpage@pgadmin.org> wrote:
>> >>
>> >> Hi
>> >>
>> >> On Fri, Jan 13, 2017 at 6:50 AM, Surinder Kumar
>> >> <surinder.kumar@enterprisedb.com> wrote:
>> >> > Hi
>> >> >
>> >> > Please find updated patch following changes:
>> >> > 1) Keep field 'opclass' combo box enabled.
>> >>
>> >> That doesn't seem to be working.
>
> It is now fixed in the updated patch.
>>
>> >
>> > Is the field 'opclass' is not enabled for you?.
>>
>> No.
>>
>> > Can you please tell in which scenario it doesn't gets enabled? so that I
>> > can
>> > fix it.
>>
>> PostgreSQL 9.4, table:
>>
>> CREATE TABLE public.foo
>> (
>>     id text COLLATE pg_catalog."default" NOT NULL,
>>     data1 text COLLATE pg_catalog."default",
>>     CONSTRAINT foo_pkey1 PRIMARY KEY (id),
>>     CONSTRAINT gerp1 UNIQUE (id, data1)
>> )
>>
>> 1) Right-click, Create index
>>
>> 2) Name: xxxx
>>
>> 3) Access method: gist
>>
>> 4) Add column. Select id (note the opclass field is not enabled)
>>
>> 5) Add column. Select data1 (note the opclass field is not enabled).
>>
>> 6) Click save, and behold the following error in all it's glory :-)
>>
>> ERROR: data type text has no default operator class for access method
>> "gist"
>> HINT: You must specify an operator class for the index or define a
>> default operator class for the data type.
>
> Thanks for steps.
>>
>>
>> --
>> Dave Page
>> Blog: http://pgsnake.blogspot.com
>> Twitter: @pgsnake
>>
>> EnterpriseDB UK: http://www.enterprisedb.com
>> The Enterprise PostgreSQL Company
>
>



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

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