search

kaleidos / grails-postgresql-extensions

Grails plugin to use postgresql native elements such as arrays, hstores,...
Apache License 2.0
78 stars 63 forks source link

Getting error for ArrayType when I use dbCreate = 'validate' #87

Closed andrebrov closed 7 years ago

andrebrov commented 8 years ago

Hi, In one of my domain class I have next fiels pageKeywords type: ArrayType, params: [type: String] When I use dbCreate = 'validate' option in data source config I'm getting next error:

Wrong column type in public.sc_page for column page_keywords. Found: _varchar, expected: varchar[]

ilopmar commented 8 years ago

Thank you for submitting the issue. I'll try to test it fix it.

sean7512 commented 8 years ago

I am also seeking the same behavior.

I overrode the dialect class to register a String Array to text so my error message is:

Found _text, expected: text[]

UPDATE: I fixed it for now, by changing the 2nd parameter of registerColumn to _text Functionality appears to work fine with this change.

gtors commented 8 years ago

From PostgreSQL documentation:

Array Types

Whenever a user-defined type is created, PostgreSQL automatically creates an associated array type, whose name consists of the base type's name prepended with an underscore, and truncated if necessary to keep it less than NAMEDATALEN bytes long.

donbeave commented 8 years ago

Adding registerColumnType(ArrayType.STRING_ARRAY, "_varchar"); in PostgresqlExtensionsDialect can fix this bug.

karaken12 commented 7 years ago

I have a fix for this (thanks, @donbeave!) but foolishly I committed my previous change to master, rather than a branch, so the diff includes some UUID stuff. Anyway, check karaken12/grails-postgresql-extensions@3b55bc13ca1e179465283fbf9d825b3c762c0062 to see the changes.

donbeave commented 7 years ago

@karaken12 can you do PR to this repository?

karaken12 commented 7 years ago

@donbeave Sure, I'll give it a go. It won't apply cleanly, though, as I mentioned.

Edit: Oh, apparently pull requests don't work the way I thought. :-( So, the changes I made are lumped in with my earlier PR for UUID array support. I'll make a new branch and PR that instead.

donbeave commented 7 years ago

I think you can do one PR with this UUID support, looks good to me.

sean7512 commented 7 years ago

Looks good...not really part of this issue, but would be nice to also support _text instead of only _varchar

karaken12 commented 7 years ago

Right, created a separate PR for this issue, which should apply cleanly. I already created a PR for UUID support: PR #93.

karaken12 commented 7 years ago

@sean7512 It's pretty easy to change it so it supports text[] instead of varchar[], but I'm not so sure how to support both. Is there a big difference between them?

ilopmar commented 7 years ago

Thank you very much for the PRs. I'll try to review them during the following days but I can't promise anything because I'm really busy these days.

sean7512 commented 7 years ago

@karaken12 yes, its easy...I overrode it for my project as the existing database on my project uses text[], but it would be nice to be able to support both at the same time. I easily see a scenario where a database has a mix of them for some unknown reasons.

ilopmar commented 7 years ago

This change was included in #93 that is now merged on master. I've released the version 4.6.7 with this change.