[ 
https://issues.apache.org/jira/browse/OPENJPA-3014?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18126676#comment-18126676
 ] 

ASF subversion and git services commented on OPENJPA-3014:
----------------------------------------------------------

Commit 71b88653553ad0c306e9f7efb3c8876c945e6357 in openjpa's branch 
refs/heads/OPENJPA-3014 from Richard Zowalla
[ https://gitbox.apache.org/repos/asf?p=openjpa.git;h=71b886535 ]

[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.


> 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
>            Priority: Major
>
> 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)

Reply via email to