I'm working on improving support for Firebird in Hibernate, and I have run into a problem where org.hibernate.tool.schema.internal.SchemaCreatorImpl is always trying to create a pooled sequence. Specifically: applySqlStrings( dialect.getCreateSequenceStrings( jdbcEnvironment.getQualifiedObjectNameFormatter().format( sequence.getName(), dialect ), sequence.getInitialValue(), sequence.getIncrementSize() ), formatter, options, targets ); With the default implementation this results in a org.hibernate.MappingException: org.hibernate.dialect.FirebirdDialect does not support pooled sequences (or at least: not by specifying it in the DDL). I'd like to know if I should just override protected String getCreateSequenceString(String sequenceName, int initialValue, int incrementSize) and ignore the initialValue and incrementSize and just return the result of getCreateSequenceString(sequenceName) or if I should change the above code to call the deprecated(!) method getCreateSequenceStrings(String sequenceName) is supportsPooledSequences() is false: dialect.supportsPooledSequences() ? dialect.getCreateSequenceStrings( jdbcEnvironment.getQualifiedObjectNameFormatter().format( sequence.getName(), dialect ), sequence.getInitialValue(), sequence.getIncrementSize() ) : dialect.getCreateSequenceStrings( jdbcEnvironment.getQualifiedObjectNameFormatter().format( sequence.getName(), dialect ) ), Kind regards, Mark Rotteveel -- Mark Rotteveel _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

On 28-7-2016 12:22, Vlad Mihalcea wrote: > Hi Mark, > > I've just run the PooledHiLoSequenceIdentifierTest test after modifying > the TABLE to SEQUENCE to see how it works and this is what I found: > > 1. SchemaCreatorImpl calls String[] getCreateSequenceStrings(String > sequenceName, int initialValue, int incrementSize) > 2. The String getCreateSequenceString(String sequenceName, int > initialValue, int incrementSize) is then called which does this: > > if ( supportsPooledSequences() ) { > return getCreateSequenceString( sequenceName ) + " start with " + > initialValue + " increment by " + incrementSize; > } > throw new MappingException( getClass().getName() + " does not support > pooled sequences" ); > Yes, and the above exception is my problem, however this default implementation suggests that this method should only be called if pooled sequences are supported (quod non), but in the current code it is called always in org.hibernate.tool.schema.internal.SchemaCreatorImpl. Now, this method is overridden in PostgreSQL and HSQLDB dialects. > Yes, but both support pooled sequences. I think we should create the sequence generator as indicated in this > Firebird documentation: > > http://firebirdsql.org/manual/generatorguide-sqlsyntax.html > The current dialect already does that, but it isn't working because of above code. Providing the following override does seem to work, but this seems at odds with the exception thrown in the default implementation. @Override protected String getCreateSequenceString(String sequenceName, int initialValue, int incrementSize) throws MappingException { // Ignore initialValue and incrementSize return getCreateSequenceString( sequenceName ); } For Firebird 2.5, http://www.firebirdsql.org/file/documentation/reference_manuals/fblangref... is more up-to-date. I'm currently creating a new Firebird 2.5 specific dialect, and then I'm going to add a Firebird 3.0 dialect. I might clean up the old InterbaseDialect and FirebirdDialect, but I don't want to touch those too much (especially since Firebird and Interbase should now be considered two distinct databases). Shouldn't we use GEN_ID for the pooled optimizers? > Yes GEN_ID could be used for that, but as far as I can tell the current support for pooled sequences seems to require specifying the increment size in the sequence DDL, which is not supported by Firebird. Is there a specific extension point to override this, is it the getDefaultMultiTableBulkIdStrategy()? Mark -- Mark Rotteveel

From there, either:

On 28-7-2016 13:02, Vlad Mihalcea wrote: > Is it that you get the exception even if the incrementSize is 1? I guess > this is an issue that we need to open a Jira ticket for it. Yes, see the code in my initial email: SchemaCreatorImpl always tries to create a pooled sequence. Mark -- Mark Rotteveel _______________________________________________ hibernate-dev mailing list hibernate-dev(a)lists.jboss.org https://lists.jboss.org/mailman/listinfo/hibernate-dev

I do think this is an error. I think the proper fix is to first make use of Exporter<Sequence>#getSqlCreateStrings via Dialect#getSequenceExporter. From there, either: 1. Change the standard Exporter<Sequence> to look at Dialect#supportsPooledSequences and deciding which Dialect#getCreateSequenceStrings form to call 2. Change Firebird's Exporter<Sequence> impl

I'd also suggest we properly deprecate Dialect#supportsPooledSequences, Dialect#getCreateSequenceStrings directing to Dialect#getSequenceExporter.

Moving this functionality into the Exporter is the correct answer. Eventually those DIalect methods will go away. On Sat, Jul 30, 2016 at 3:32 AM Mark Rotteveel <mark(a)lawinegevaar.nl&gt; wrote: > On 28-7-2016 18:07, Steve Ebersole wrote: > > I do think this is an error. I think the proper fix is to first > > make use of Exporter<Sequence>#getSqlCreateStrings via > > Dialect#getSequenceExporter. > > > > From there, either: > > > > 1. Change the standard Exporter<Sequence> to look at > > Dialect#supportsPooledSequences and deciding which > > Dialect#getCreateSequenceStrings form to call > > 2. Change Firebird's Exporter<Sequence> impl > > I have fixed the immediate problem by implementing the > getCreateSequenceStrings(String, int, int) and > getCreateSequenceString(String, int, int) as: > > @Override > public String[] getCreateSequenceStrings(String sequenceName, int > initialValue, int incrementSize) throws MappingException { > return new String[] { > getCreateSequenceString( sequenceName, initialValue, > incrementSize ), > "alter sequence " + sequenceName + " restart with " + > (initialValue - 1) > }; > } > > @Override > protected String getCreateSequenceString(String sequenceName, int > initialValue, int incrementSize) > throws MappingException { > if (incrementSize > 1) { > throw new MappingException( getClass().getName() + " does not > support pooled sequences" ); > } > // Ignore initialValue and incrementSize > return getCreateSequenceString( sequenceName ); > } > > So as long as a non-pooled sequence (incrementSize = 1) is requested the > Firebird25Dialect will happily oblige. I will add a similar solution to > the InterbaseDialect and FirebirdDialect. > > > I'd also suggest we properly deprecate Dialect#supportsPooledSequences, > > Dialect#getCreateSequenceStrings directing to > Dialect#getSequenceExporter. > > The problem with that is that the StandardSequenceExporter uses > getCreateSequenceStrings to do its work, so deprecating that would > require more rework. > > Mark > -- > Mark Rotteveel >