hibernate-ogm-ignite
hibernate-ogm-ignite copied to clipboard
hibernate
use an index name that is guaranteed to be no more than 255 chars
Ignite indices have a char limit of 255 on the name. The current naming scheme easily exceeds that in complex models because it uses the field names on the entity. This PR uses a random UUID instead, ensuring that limit is never hit. I considered using "tableName + UUID" but that just makes it less likely since it's possible to have entity names longer than the 127 chars that the UUID leaves us with.
For reference an example of the current scheme failing manifests itself as in the attached file/stacktrace. ignite-ogm-index-name-error.txt
Can one of the admins add this person to the trusted builders? (reply with: "add to whitelist" or "ok to test")
Hi @zcourts !
Thanks for PR! But ... before merge the PR ... Could you please create a issue in project's jira ? Project's Jira URL is http://hibernate.atlassian.net
Also ... Do you use the dialect for your project? What is your impressions? Thanks for feedback!
Hi @schernolyas I've created https://hibernate.atlassian.net/browse/OGM-1507
We're migrating an in-house ORM-like library we built to do this with Ignite. So far it has been ok. No major issue or complaint. Having never used Hibernate ORM, or OGM at all before this project, it's been more straightforward than I'd expect. The ignite dialect itself I've only just switched over to using, the initial spike was to investigate only and I did the spike using the Infinispan embedded dialect. The switch over was quick, having learnt what was needed using Infinispan.
The docs are a pain point, If I hadn't done Infinispan first I suspect it'd be a lot harder to use this dialect. The asci docs in the documentation module was invaluable in learning what configs were available to start.
Having the ability to choose an existing Ignite instance started earlier in the JVM was crucial, I'd have had to add that option if it wasn't already present. We have a huge amount of infrastructure already built around Ignite and customising various pieces.
My initial test works fine, but having switched to the patch branch I'm actually now getting an IndexOutOfBoundsException, value.getColumnIterator() in https://github.com/hibernate/hibernate-ogm-ignite/blob/master/ignite/src/main/java/org/hibernate/ogm/datastore/ignite/impl/IgniteCacheInitializer.java#L284 returns more entries than ( (ComponentType) type ).getSubtypes().
I'm investigating, will either report a bug later or another PR/fix if it's not my fault.
My next point of call is going to be investigating how I can provide a custom CacheConfiguration. I've already found https://github.com/hibernate/hibernate-ogm-ignite/blob/master/ignite/src/main/java/org/hibernate/ogm/datastore/ignite/impl/IgniteCacheInitializer.java#L196 but I'm still trying to find my way around the code so it's unclear at the moment how I'll approach this but definitely need to customise the cache configs. Potentially another PR (I keep thinking there's probably a means of exposing dialect specific stuff but haven't found it. I know about unwrap but haven't investigated as a potential route yet)
It's still early days (though I'm hoping a migration to this is completed and ready for integration tests next week). Once I've spent more time, happy to provide feedback.
EDIT:
- While I'm at it, is there a reason #19 hasn't been merged?
- The version in the pom is 5.3.0-SNAPSHOT but
5.3.1-Finalis already in the tree. Is that intentional?
While I'm at it, is there a reason #19 hasn't been merged?
There were some conflicts when I review it the first time and then I went on Holiday. I just came back going through the accumulated emails and new PRs.
The version in the pom is 5.3.0-SNAPSHOT but 5.3.1-Final is already in the tree. Is that intentional?
I will check, it seems an error. Thanks for telling us
@zcourts I left a few comments about this PR, thanks a lot for your help.
Feel free to ask us for further questions if you something is not clear. Cheers
Agree with @DavideD. Index names should be more readable.
For example, if length > 255, you take first N chars and append an autoincremented number to ensure uniqueness, like it's done for aliases in queries:
TABLE_NAME_VERY_LONG...._COL_001
TABLE_NAME_VERY_LONG...._COL_002
etc.
If length <= 255, it would be left uncnahged.
That would work, a simpler solution could be to throw an exception at startup and let the user rewrite the default name. I don't think situation will happen often
In general, we prefer to give the user enough information so that it can easily solve issues based on what's best for him.
@DavideD agree. A better way would be to throw an exception proposing to reduce table/column names or define this index manually in @Table(indexes =...).
That's fair enough. I'll add a test case for > 255 chars raising an exception and a success case for using javax.persistence.@Index to manually name the index. Will update the PR to address the comments raised.
Hi @schernolyas I was busy that week and this slipped off my radar. I just attempted the fix with some tests. It needs a bit of guidance - see the questions I've raised above
Didn't realise "PENDING" meant I had to submit before anyone else could see the comments
That's great feedback, thank you and thanks for the pointer to that file. I'm out of the office until Tuesday, I'll take a look at all of the points you've raised and address them next week.