Skip to content

Loading…

DBAL-730: [GH-464] [DBAL-139] [WIP] Finish CACHE option implementation for sequences #1956

Closed
doctrinebot opened this Issue · 0 comments

3 participants

@doctrinebot

Jira issue originally created by user @doctrinebot:

This issue is created automatically through a Github pull request on behalf of deeky666:

Url: #464

Message:

@beberlei Started implementation for DBAL-139 in commit e8efcd7 and d397c00 which was first drafted in PR #202. It's about adding a cache option for sequences.

The following missing aspects of the implementation were added:

  • SQL Server support
  • SQL Anywhere support
  • Reverse engineering the cache option with the schema managers
  • Comparator implementation
  • Schema::createSequence() adjustment
  • More tests

Additional optimizations:

  • Rename $cache to $cacheSize in Sequence (unified naming with $allocationSize)
  • More strict input validation for cache size values in Sequence to avoid SQL generation errors
  • Better API documentation
  • Unified getSequenceCacheClause() method on all platforms

Todo:

  • Fix comparator issues (see below)
  • Finish test suite for this (depends on comparator issues solution)

Comparator issues
The comparator will get into trouble when comparing cache size values. To get an understanding of this I'll first explain how cache size values evaluate to the database:

  • null -> No cache size specified, don't generate cache clause for sequences and use platform's default
  • 0 -> Explicitly disable caching for sequences, generates NOCACHE clause on platforms
  • every other positive value is an explicite specification of the cache size and results in CACHE <size> clause

Now there are two problems.

First one is that a value of 1 has a special meaning on some platforms, too. Oracle does not accept it and throws an error as the minimum allowed value is 2. PostgreSQL allows a value of 1 but in fact interprets it as NOCACHE. SQL Server and SQL Anywhere accept a value of 1 but the behaviour is unclear compared to an explicite NOCACHE on these platforms. Now I am assuming that a cache size of 1 maybe also physically has the same effect as if specifying no cache at all and that this is more of a "compatibility" value and we maybe should not allow it at all to preserve portability throughout the vendors.

The second problem is the null value for $cacheSize in Sequences. This tells the vendor to use it's own specific default cache size which differs on all platforms. The comparator will always mark a sequence as changed then when null was initially defined for the sequence and the sequence is introspected from database again. This will lead to repeated ALTER SEQUENCE statements in the schema tool which is not good. Also we cannot determine in the platform itself if the sequence has really changed as there currently is no SequenceDiff class which could get passed to AbstractPlatform::getAlterSequenceSQL(). Instead we only get the changed sequence object. I have put together some examples to clarify on what I mean as it is hard to explain.

So I'm asking for opinions and input on this :)

@doctrinebot doctrinebot added the Bug label
@beberlei beberlei was assigned by doctrinebot
@deeky666 deeky666 added Duplicate and removed Bug labels
@beberlei beberlei was unassigned by deeky666
@deeky666 deeky666 closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.