NH-3921 - Sequence support for Ingres9

[oskarb]

Cleaned up version of PR #439.
https://nhibernate.jira.com/browse/NH-3921
Fixes #768

[oskarb]

For 5.0 since it might change behaviour.

[oskarb]

The failing test is DialectSupportingSequencesMustFullfillSequenceContract().

[hazzik]

Dialect is missing implementations for GetCreateSequenceString and GetDropSequenceString

[hazzik]

@oskarb shall we install Ingres on a build agent?

[oskarb]

@hazzik I think the main concern about testing on Ingres or not is how much we should care about test failures, particularly in new test cases which are not specifically about Ingres.

It does slow us down to fix failures for specific dialects, given that often it's not really a problem in NHibernate itself but rather that the test (accidentally) tries to use something not supported in a specific dialect or database engine, or there is something silly like a rounding difference. On the other hand, it does obviously lead to better quality if the dialects are tested and problems fixed immediately.

What's the state of Ingres? With regards to what we test on today, it seems we are more likely to have cumbersome failures on platforms with a limited feature set (quite naturally).

[fredericDelaporte]

On Hibernate side, they have put the sequence support on the base class IngresDialect, not on the Ingres9Dialect. On Actian site, I do not find documentation for a version prior to the 10, so I have not checked Hibernate is right assuming Ingres supports sequence even before its 9 version.
Maybe should we anyway align on Hibernate for this.

QuerySequencesString is not overridden though seems documented as required if sequences are supported (formulated the other way round indeed: "The select command; or null if sequences are not supported.")
Hibernate implementation for this is on IngresDialect return "select seq_name from iisequence";, overridden again on Ingres9Dialect for appending a s: return "select seq_name from iisequences";. Ingres11 documents the former as a system catalog, and the later as a standard catalog

Beginning with Ingres9, Hibernate declares it supports pooled sequences. We should override SupportsPooledSequences in Ingres9 dialect then. (No need to override GetCreateSequenceString(string, int, int) overload, its default implementation is suitable according to Ingres11 documentation and Hibernate implementation.

[fredericDelaporte]

For information, Hibernate use the alternate syntax: sequenceName + ".nextval". No indication in Ingres 11 documentation (nor in 10) if one of these syntaxes is supported since longer.

I am not sure Hibernate is a good source for this case though, since their GetSequenceNextValString syntax is not even documented in previous links (they do not call GetSelectSequenceNextValString, they return "select nextval for " + sequenceName

The way Ingres documentation is written let me think the "main" syntax is the one currently in this PR.
So for me, the syntax in this PR is good.

[fredericDelaporte]

Hibernate add after sequenceName the keyword restrict. Though documented only for other types of objects, it would be there for ignoring the drop if the sequence is referenced by another object in the database.

A bit conservative I think, and not done for other dialects, so I think above syntax is ok as currently is.

[fredericDelaporte]

Rebased, and added a commit with my proposed minimal changes.

[fredericDelaporte]

I realize a bit late this one should have been re-planed for 6.0, not 5.1, because it changes the native, identity and enhanced-sequence generator behavior. With Ingres9Dialect, they will all start using a sequence. I may avoid the change for native and identity by PR an override of IdentityStyleIdentifierGeneratorClass and NativeIdentifierGeneratorClass in Ingres9Dialect, with a "6.0 TODO: remove". But for the enhanced-sequence generator, the only solution seems to tell users to add in its mapping the force_table_use parameter. So it will still be a possible breaking change.

So either we revert this on master, or we accept this possible breaking change.

[hazzik]

So either we revert this on master

It's not possible to undo mince.

I'm voting to the option of overriding native and identity and possible breaking changes with force_table_use

[fredericDelaporte]

So either we revert this on master

It's not possible to undo mince.

It is still doable with a revert PR, but adding noise in history. I was thinking about this option.

I'm voting to the option of overriding native and identity and possible breaking changes with force_table_use

Ok, will do.