[
https://issues.apache.org/jira/browse/OPENJPA-3014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18128577#comment-18128577
]
ASF subversion and git services commented on OPENJPA-3014:
----------------------------------------------------------
Commit 317ed8f8c0d7b4044dc9eeff29a57f9e1ee9a2d9 in openjpa's branch
refs/heads/OPENJPA-2951 from Richard Zowalla
[ https://gitbox.apache.org/repos/asf?p=openjpa.git;h=317ed8f8c ]
[OPENJPA-3014] Back a named sequence generator by a table, and use real
sequences on MariaDB (#217)
* [OPENJPA-3014] Back a named sequence generator by a table where the database
has none
A named @SequenceGenerator failed outright on every database whose
dictionary defines no NextSequenceQuery. AnnotationPersistenceMetaDataParser
sets the plugin of such a generator to SequenceMetaData.IMPL_NATIVE unless
the sequence name is itself a plugin string, JDBCSeqValue maps that alias to
NativeJDBCSeq, and NativeJDBCSeq.endConfiguration() refuses to configure
without a NextSequenceQuery:
ArgumentException: Error instantiating named sequence "SEQGENERATOR":
Your database dictionary does not support native sequences
openjpa.Sequence cannot help, because SequenceMetaData.instantiate()
consults the configured system sequence only for the system sequence name,
so a named generator always takes the plugin path. There was no property
that avoided the failure.
The report names MySQL, found while running the TCK, but the dictionaries
without a NextSequenceQuery include Apache Derby, so the failure also
reproduces on the database the default test profile uses; the new test
fails on Derby, H2 excepted, before this change. They also include
AbstractSQLServerDictionary, SQLServerDictionary, SybaseDictionary,
MaxDBDictionary, SolidDBDictionary, HerdDBDictionary, AccessDictionary and
InterbaseDictionary, and SQL Server has had sequence objects since 2012,
so there the fallback stands in for a sequence the database could have
provided. The migration notes name those databases and say that
configuring NextSequenceQuery is how a generator keeps drawing from a
sequence that already exists, with a pointer from the SQL Server section.
Jakarta Persistence 3.1 section 11.2.4 and the GeneratedValue strategies in
section 11.1.21 define how a generator declaration is mapped, and
GenerationType.SEQUENCE says "the persistence provider must assign primary
keys for the entity using a database sequence", but the specification
elsewhere leaves the realisation of a generator to the provider, which is
why providers emulate sequences where a database has none. The observable
contract is that identifiers are unique and that initialValue and
allocationSize are honoured. Resolving a named generator to a table backed
sequence therefore raises no conformance obstacle; it is a design decision
with a compatibility dimension, not something the specification demands.
SequenceMapping.instantiate() now substitutes a value table sequence when
the plugin is a plain request for a native sequence and
dict.nextSequenceQuery is null, gated on the dictionary's own property
rather than on a dictionary class so that a user supplied NextSequenceQuery
keeps the native path. The substitution sits where the sequence is
resolved, so the runtime and the mapping tool agree - a patch proposed on
the dev list changed only MappingTool.addSequenceComponents, which leaves a
plain EntityManagerFactory failing and works by mutating shared metadata.
Instead of mutating this SequenceMapping, which is shared, the fallback
builds a separate metadata instance and instantiates that; the substitution
is logged as a warning, once per generator, because the sequence instance is
cached on the metadata.
ValueTableJDBCSeq rather than TableJDBCSeq: ValueTableJDBCSeq keys its
counter row by a name, so the row is keyed by the sequence name the
generator asked for and entities sharing one generator share one counter,
as they would with a real sequence. TableJDBCSeq keys the row per class and
would give each entity an independent counter, which is neither the
annotation's semantics nor what the native path does. The sequence name is
qualified with the catalog and schema of the sequence, so that two
sequences of the same name in different schemas stay distinct without the
internal table having to live in a schema the generator meant for a
sequence object. That schema is the one a sequence object would have been
created in: the generator's own, or, when it declares none,
the persistence unit default that SequenceMetaData.setSchema substitutes.
The emulated counter is therefore scoped exactly as the sequence it stands
in for, which the Javadoc, the migration notes and a test with a qualified
generator now say.
initialValue and allocationSize travel on the SequenceMetaData rather than
in the plugin string and are copied onto the substitute, which the test
asserts both on the resulting Seq and through the identifiers it hands out,
by counting the statements that advance the generator's counter: one
allocation per block of allocationSize, not one per identifier.
The increment is deliberately not copied: TableJDBCSeq.setIncrement is a
deprecated alias for setAllocate and, appended after Allocate, would
overwrite the allocation size.
openjpa.Compatibility=EmulateNativeSequences=false restores the hard
failure. The default is the fallback, because the behaviour it replaces is
an unconditional failure with no workaround, so nothing that works today
changes; a schema built by an earlier release will simply gain the sequence
table.
A dictionary identified from the connection URL alone has not seen a
connection when the generator is resolved, and some dictionaries only learn
from the server version whether the database has sequences, so where the
dictionary itself declares no NextSequenceQuery the gate asks the
configuration for its connection factory, which installs the dictionary's
connected configuration. Otherwise which sequence backs a generator would
depend on whether anything else had touched the database first. The cheap
check comes first, so a dictionary that sets NextSequenceQuery in its
constructor - Oracle, DB2, PostgreSQL, H2, HSQL, Informix, Firebird,
Ingres - is still resolved without a connection and an offline mapping tool
run against such a configuration does not stall on a connect attempt.
DBDictionary.isConnected() exposes the connected state.
If that connect attempt fails, the generator fails to resolve, with the
connect failure as the cause of a MetaDataException, rather than falling
back. Guessing cannot help there - the table fallback advances its counter
over a connection of its own, so nothing hands out an identifier without
one either way - and it can harm: SequenceMetaData.getInstance caches the
Seq, so the guess holds for the life of the factory, and a node of a
clustered deployment whose JNDI lookup happened to fail would then hand out
identifiers from a counter the other nodes do not use. That is the same
divergence the sibling commit avoids by parsing the version prefix, and a
log warning is not a sufficient signal for it. A missing driver class
arriving as a LinkageError is wrapped alongside the runtime exceptions.
The test asks the dictionary which kind of database it is on rather than
assuming, because both kinds exist and the same observable contract has to
hold on both. Whether a database has sequence objects is
DBDictionary.nextSequenceQuery, which a dictionary may only set once it has
seen the server, so the test connects it exactly as the fallback above
does - DBDictionary.isConnected(), then JDBCConfiguration
.getConnectionFactory() - before reading it; reading it off an unconnected
dictionary would expect the fallback on a server that has sequences. The
unique identifiers, the initialValue and the allocation count are asserted
on both paths. Only which Seq implementation backs the generator, and that
EmulateNativeSequences=false brings the old failure back, are specific to
the fallback; the latter is skipped with a message naming the platform
where the database has sequences. The allocation count is measured as the
number of statements that advance the counter, which is the UPDATE of the
counter row on the fallback and the dictionary's NextSequenceQuery - for
instance SELECT NEXTVAL(OJPA3014_SEQ) - on the native path, with the
CREATE SEQUENCE and the ALTER SEQUENCE ... INCREMENT BY excluded, since
neither hands out a value.
On a database that has sequence objects that test only re-asserts the
native path: of the five cases, four hold on master there as well, and only
the Derby half of the CI matrix sees the substitution end to end. The gate
itself is therefore covered server free by the new
TestSequenceMappingFallback in openjpa-jdbc, which drives
SequenceMapping.instantiate() with a stub dictionary and a stub DataSource
on every platform: a dictionary that learns of sequence support only in
connectedConfiguration() reaches NativeJDBCSeq, one that never declares any
reaches ValueTableJDBCSeq with the sequence name, initialValue and
allocationSize carried over, one that declares NextSequenceQuery in its
constructor is resolved without the data source being touched at all, a
data source that cannot connect makes the resolution fail with its own
SQLException reachable in the cause chain, and
EmulateNativeSequences=false brings the historical failure back.
* [OPENJPA-3014] Use real sequences on MariaDB 10.3 and later
MariaDB 10.3 introduced sequence objects - CREATE SEQUENCE, NEXTVAL(),
NEXT VALUE FOR - but MariaDBDictionary still declared no
NextSequenceQuery, so every @SequenceGenerator was treated as
unsupported. Set it in connectedConfiguration() once the server reports
10.3 or later, the way PostgresDictionary version gates
storeCharsAsNumbers. Older servers keep the table sequence fallback.
MariaDBDictionary.java:57 reads "public class MariaDBDictionary extends
DBDictionary": the dictionary is a copy of the MySQL one, not a subclass
of it, down to a private getMajorMinorVersions of its own at line 244.
Nothing here is inherited, and MySQLDictionary is untouched.
There is no setter for NextSequenceQuery and hence no flag recording that
the user configured one, and this change does not invent one: this
dictionary leaves the field null in its constructor and
DBDictionary.endConfiguration() nulls an empty value out, so a value seen
in connectedConfiguration() was configured explicitly and is left alone.
A MariaDB 10.x server reports its version to a client that is not MariaDB
aware with a replication compatibility prefix - "5.5.5-10.3.39-MariaDB"
rather than "10.3.39-MariaDB". The prefix was dropped in 11.0, so an 11.x
server reports "11.8.9-MariaDB-ubu2404" to any client; both forms were read
off the handshake greeting of mariadb:10.3 and mariadb:11.8 to confirm it. Both
forms reach
this dictionary, but not by the same route: DBDictionaryFactory
.newDBDictionary selects on getDatabaseProductName(), falls back to the URL
protocol, and looks at getDatabaseProductVersion() only when the name maps
to MySQL (lines 93-102). With mariadb-java-client the product name is
already "MariaDB" and the version string plays no part in the selection;
the prefixed form matters because MySQL Connector/J reports the product
name as "MySQL", so for that client the version string is both what selects
this dictionary and what carries the prefix. What the version parsing has
to cope with is the prefix itself, because it stops at the first character
that is neither a digit nor a dot: on the prefixed form it would read 5.5
and withhold sequence support from an 11.8 server reached through a driver
that passes the handshake version through, while the same server reached
through mariadb-java-client used a sequence object - two deployments of one
application handing out identifiers from two independent counters. Strip
the prefix before parsing.
The strip moves every feature this dictionary gates on the version, not
only sequence support. The pre-existing gate for 10.2 and later
(maj > 10 || (maj == 10 && min > 1)) sets
timestampTypeName="DATETIME{0}", moves it into fractionalTypeNameSet and
sets dateFractionDigits=6, so a server reported as "5.5.5-10.2.44-MariaDB"
now gets microsecond DATETIME columns where it was parsed as 5.5 and got
whole-second DATETIME before - the same DDL the same server already got
through mariadb-java-client. TestMariaDBDictionary pins both forms of a
10.2 version to DATETIME{0}/6 and a genuine 10.1 version to DATETIME/0,
and the MariaDB section of the migration notes says that such a deployment
now generates microsecond columns, that a schema from an earlier release
keeps its whole-second ones, and that the gate assigns both fields
outright, so only a subclass restores the previous types.
Declaring sequence support means the schema tool now issues CREATE SEQUENCE
and DROP SEQUENCE for these generators, which brings two more things with
it:
- MariaDB caches 1000 increments of a sequence per server by default and
discards what is left of that range whenever the cache is flushed, which
an ALTER SEQUENCE or a restart does. OpenJPA caches values itself, in
blocks of the generator's allocation size, and NativeJDBCSeq issues
ALTER SEQUENCE ... INCREMENT BY once per sequence instance to do so,
which is itself such a flush, so the server side cache adds nothing but
those gaps. getCreateSequenceSQL() therefore creates sequences with
NOCACHE. Measured on MariaDB 11.8 against a sequence created START WITH
100 INCREMENT BY 7: SELECT NEXTVAL returns 100 and leaves
NEXT_NOT_CACHED_VALUE at 7100, and after ALTER SEQUENCE ... INCREMENT BY
7 the next SELECT NEXTVAL returns 7100 - 1000 increments skipped, not
1000 values. With NOCACHE the same statements return 100 and then 107.
The statements OpenJPA emits for a generator with initialValue 100 and
allocationSize 7, as the SQL that TestNamedSequenceGenerator collects
shows them on MariaDB 11.8, are
CREATE SEQUENCE OJPA3014_SEQ START WITH 100 INCREMENT BY 7 NOCACHE
ALTER SEQUENCE OJPA3014_SEQ INCREMENT BY 7
SELECT NEXTVAL(OJPA3014_SEQ)
- unquoted, and with no CYCLE clause. The 7 in INCREMENT BY is
increment * allocate, which DBDictionary.commonCreateAlterSequenceSQL
multiplies out; NativeJDBCSeq leaves the increment at its default of 1,
so for this generator it is the allocation size. In that order no jump
occurs, the ALTER arriving before the first NEXTVAL has cached anything,
and 15 identifiers cost three NEXTVALs; the jump is what a later
EntityManagerFactory would see, issuing the ALTER against a sequence
whose values have been cached, and that is the case NOCACHE covers.
- Reflecting on the schema to find existing sequences needs a query, since
DatabaseMetaData does not report them. MariaDB has no
INFORMATION_SCHEMA.SEQUENCES before 10.6, but a sequence appears in
INFORMATION_SCHEMA.TABLES with a TABLE_TYPE of SEQUENCE in every version
that has sequences, so getSequencesSQL() reads it from there. The schema
name is matched case insensitively because DBDictionary.getSequences()
upper cases the schema it passes, while MariaDB keeps the case a schema
was created with. INFORMATION_SCHEMA spans every database on the server,
though, and every caller asks for a null schema - SchemaGenerator
.generateSequences() passes DBIdentifier.NULL deliberately, and an
unqualified @SequenceGenerator has none - so the unqualified case is
scoped to DATABASE(). Unrestricted it would report the sequences of other
applications' databases, which makes SchemaTool.buildSchema() skip a
CREATE SEQUENCE it owes, because SchemaGroup.findSequence() matches any
schema for an unqualified sequence, and makes a retain, refresh or dropDB
action issue DROP SEQUENCE outside the configured database. DATABASE()
takes no bind parameter, so the parameter indices getSequences() binds
stay as they were.
TestMariaDBDictionary drives connectedConfiguration() with a reflection
proxy DatabaseMetaData, as TestPostgresDictionary does for OPENJPA-2971, so
the version gate, the survival of an explicitly configured
NextSequenceQuery, the generated temporal types, the NOCACHE suffix and the
predicates and binding order of the sequence query are covered without a
server: 18 tests, all passing. TestNamedSequenceGenerator passes on Derby
and H2, the two profiles CI runs, and passed on MariaDB 10.2 and 11.8 and
MySQL 8.0 when it was measured there, which is where the SQL quoted above
was collected.
---------
Co-authored-by: Maxim Solodovnik <[email protected]>
> A named @SequenceGenerator fails where the database has no sequences
> --------------------------------------------------------------------
>
> Key: OPENJPA-3014
> URL: https://issues.apache.org/jira/browse/OPENJPA-3014
> Project: OpenJPA
> Issue Type: Bug
> Components: jpa
> Reporter: Richard Zowalla
> Assignee: Richard Zowalla
> Priority: Major
> Fix For: 4.2.0
>
>
> A named @SequenceGenerator fails on any database whose dictionary has no
> nextSequenceQuery, for example MySQL:
> org.apache.openjpa.persistence.ArgumentException: Error instantiating named
> sequence "SEQGENERATOR": Your database dictionary does not support native
> sequences.
> at
> org.apache.openjpa.jdbc.kernel.NativeJDBCSeq.endConfiguration(NativeJDBCSeq.java:190)
> Found by Maxim Solodovnik while running the TCK against MySQL.
> Cause: AnnotationPersistenceMetaDataParser sets the plugin of a
> @SequenceGenerator to SequenceMetaData.IMPL_NATIVE unless the sequenceName is
> itself a plugin string, and JDBCSeqValue maps that alias to NativeJDBCSeq,
> which refuses to configure when dict.nextSequenceQuery is null. The
> openjpa.Sequence setting does not help: SequenceMetaData.instantiate()
> consults the configured system sequence only for the system sequence name, so
> a named generator always takes the plugin path. No configuration property can
> therefore avoid the failure.
> Two independent parts.
> 1. Fall back to a table backed sequence where the database has none. OpenJPA
> already ships SequenceMapping.IMPL_TABLE and IMPL_VALUE_TABLE (TableJDBCSeq,
> ValueTableJDBCSeq), which keep the current value in a row of a sequence table
> and update it in place, the same approach other providers take on MySQL. A
> named generator resolved against a dictionary without nextSequenceQuery
> should use that instead of failing, with a warning, gated on
> nextSequenceQuery being null rather than on a dictionary class so every such
> dictionary benefits. The substitution belongs where the sequence is resolved
> (SequenceMapping / MappingRepository) so the runtime and the mapping tool
> behave the same; a patch that only changes MappingTool.addSequenceComponents
> fixes the tool and leaves a plain EntityManagerFactory failing, besides
> relying on a mutation of shared metadata.
> Two details to decide: IMPL_TABLE keys its rows per class, so entities
> sharing one generator would get independent counters, whereas
> IMPL_VALUE_TABLE keys by sequence name and matches @SequenceGenerator
> semantics more closely; and initialValue and allocationSize travel on the
> SequenceMetaData rather than in the plugin string, so they must survive the
> substitution.
> 2. Use the real thing on MariaDB. MariaDB 10.3 and later has sequence objects
> (CREATE SEQUENCE, NEXT VALUE FOR, NEXTVAL()), so
> MariaDBDictionary.connectedConfiguration() should set nextSequenceQuery when
> the server reports 10.3 or later, in the version gated manner
> PostgresDictionary already uses for storeCharsAsNumbers, and leave older
> servers on the fallback from part 1.
> Note for the TCK: its setup method issues CREATE SEQUENCE SEQGENERATOR START
> WITH 10 through a native query and logs the failure on MySQL. That logged
> error is expected there and harmless; what matters is that identifiers can
> still be allocated afterwards, which part 1 provides. On MariaDB the
> statement succeeds, which part 2 then uses.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
