Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

table.index() not adding SPATIAL index for geometry column #2532

Open
Frondor opened this issue Mar 19, 2018 · 6 comments
Open

table.index() not adding SPATIAL index for geometry column #2532

Frondor opened this issue Mar 19, 2018 · 6 comments

Comments

@Frondor
Copy link

Frondor commented Mar 19, 2018

Environment

Knex version: 0.14.4
Database + version: MariaDB 10.2

Issue

At creating geospatial columns, when creating the indexes, it does not create the SPATIAL index, plus getting Error: BLOB/TEXT column 'locations' used in key specification without a key length, and as far as I know, you can't set a length to MULTIPOINT

table.specificType('locations', 'MULTIPOINT').notNullable()
table.index(['locations'], null, ['SPATIAL'])
table.specificType('location', 'POINT').notNullable()
table.index(['location'], null, 'SPATIAL')

Error:

[ { sql: 'create table `merchants` (`id` int unsigned not null auto_increment primary key, `category_id` int unsigned not null, `user_id` int unsigned not null, `locations` MULTIPOINT not null, `created_at` datetime, `updated_at` datetime)',
    bindings: [] },
  { sql: 'alter table `merchants` add constraint `merchants_category_id_foreign` foreign key (`category_id`) references `categories` (`id`) on delete CASCADE',
    bindings: [] },
  { sql: 'alter table `merchants` add constraint `merchants_user_id_foreign` foreign key (`user_id`) references `users` (`id`) on delete CASCADE',
    bindings: [] },
  { sql: 'alter table `merchants` add index `merchants_locations_index`(`locations`)',
    bindings: [] } ]
[ { sql: 'drop table if exists `adonis_schema_lock`',
    bindings: [] } ]
{ Error: BLOB/TEXT column 'locations' used in key specification without a key length ... }
]
@Frondor Frondor changed the title table.index not adding SPATIAL index for geometry column table.index() not adding SPATIAL index for geometry column Mar 19, 2018
@elhigu
Copy link
Member

elhigu commented Mar 19, 2018

Which would be expected queries in this case? I suppose this one is not what you were expecting:

alter table `merchants` add index `merchants_locations_index`(`locations`)

@Frondor
Copy link
Author

Frondor commented Mar 19, 2018

Uh, forgot it, my bad:
ALTER TABLE merchants ADD SPATIAL INDEX merchants_locations_index (locations) (probably with)

@elhigu
Copy link
Member

elhigu commented Mar 21, 2018

Looks like a bug then 👍 Need to check out if that is broken in all dialects.

@Frondor
Copy link
Author

Frondor commented Apr 5, 2018

@elhigu I was giving it a try, and found out this bug has nothing to do with SPATIAL indexes, It's actually a limitation of the knex api.
As you can see here #203 (comment) (another issue that needs attention for this)
table.index(['location'], null, 'ANYTHING') is not using the index type in the query.

http://knexjs.org/#Schema-index the docs actually say you can provide the index type:

indextable.index(columns, [indexName], [indexType])
Adds an index to a table over the given columns. A default index name using the columns is used unless indexName is specified. The indexType can be optionally specified for PostgreSQL. Amazon Redshift does not allow creating an index.

But mysql dialects should also support the indexType for these use cases:

https://github.com/tgriesser/knex/blob/ee8cc35ecb41a8bc809f753b56e64691bd3fa25c/src/dialects/mysql/schema/tablecompiler.js#L160
https://github.com/tgriesser/knex/blob/ee8cc35ecb41a8bc809f753b56e64691bd3fa25c/src/dialects/mssql/schema/tablecompiler.js#L105
https://github.com/tgriesser/knex/blob/ee8cc35ecb41a8bc809f753b56e64691bd3fa25c/src/dialects/mysql/schema/tablecompiler.js#L160
https://github.com/tgriesser/knex/blob/ee8cc35ecb41a8bc809f753b56e64691bd3fa25c/src/dialects/oracle/schema/tablecompiler.js#L100

@Frondor
Copy link
Author

Frondor commented Aug 15, 2018

Any updates on this?

@elhigu
Copy link
Member

elhigu commented Aug 18, 2018

@Frondor I'm sure you would have known if you have done anything about this :) I highly doubt that anyone else is going to fix it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants