#186 [RFC][WIP] Sequence Support

[alextech]

For original thought process on this list see #186

• [x] Explicit sequence with string or array identifier
• [x] Multiple sequences per table (would love ideas for improvement)
• [x] Implicit sequence with column name only for SERIAL pseudo-type
  • [x] Generate based on PostgreSQL concatenation rule
  • [x] Query database metadata if identifier ends up being longer than 63 bytes
• [x] Add SERIAL pseudo-type to column creation DDL to compliment idea of implicit sequence at DB creation/migrations level.
• [ ] Update Insert and Update statements to filter out or generate values based on the changes
  • [ ] Integrate with #177
• [ ] Consider making SequenceFeature independent from TableGateway since (1) it does not necesseirly tie to a specific table, (2) fix RowGateway limitation
• [ ] Update Metadata TableGateway feature to recognize SERIAL columns and add SequenceFeature instances
• [ ] Add MS SQL Server 2016 support
• [ ] Stretch goal: add Oracle 12c implicit sequence support

Update 24th December 2016:

Supersedes, combines PR #162 and #172. Addresses issues brought up by @andrey-mokhov about implicit sequences. To avoid extra query, first constructing name using same naming rule as PostgreSQL and when on rare occasions exceed length, query database. Improve on original solution by using prepared statement in order to cache queries instead of recompiling what is a very common statement with only identifier name variation. Combines with @xorock solution of specifying schema together with sequence name. Parametrized statement should also deal with error prone manual value escaping.

FeatureSet filter

First, need to adjust FeatureSet to handle more than one instance of a type to categorize callMagicCall() properly.

This should help with the fact that even though TableGateway accepts unrestricted array of features, if more than one feature of the same type is added, only first one will be processed. Useful for sequences or anyone else who may have run into this.

What I am not fully happy with, could use verification on before I go too far into this:

1. Features of same type now need to have a filter of the target method to ensure they are indeed the target recipient. https://github.com/zendframework/zend-db/compare/master...alextech:sequences?expand=1#diff-85fe938f5a6f87443ad2578472eed9e0R112 This is needed so can have $tableGateway->lastSequenceId('implicit_by_column_name'); or $tableGateway->lastSequenceId('explicit_sequence_name'); calls to be delegated to appropriate sequence instance out of the array of them. Not the cleanest looking solution.

2. Is categorization by class name the best way to go? Is https://github.com/zendframework/zend-db/compare/master...alextech:sequences?expand=1#diff-a46ec23198cadca611ac2ff22cbe055dR156 the best way to loop through them? Maybe to address first point should use combination of feature class name and some unique identifier like name. But then not all features will have an obvious identifier.

[ezimuel]

Unfortunately, I've to close this PR due to inactivity.

[alextech]

@ezimuel I was waiting on some assistance about difficult architecture decisions outlined in last comment at https://github.com/zendframework/zend-db/issues/186 and a potential discrepancy with another PR linked in todo list. I got stuck not knowing how to go about those conflicts, what more experienced maintainers would do in that case, but never got reply :(

[froschdesign]

@ezimuel We should reopen this PR, because this PR is still work in progress and @alextech does a good job here. Only some feedback is needed! Maybe you can review this PR.

[froschdesign]

@ezimuel Inactivity lies on our side!

@alextech Would you complete this PR?

[ezimuel]

@alextech ok, I'm re-opening this PR. I'll try to schedule some time for review it. It's a lot of code :(

[alextech]

@froschdesign I will put the effort in! Thank you! It'll be challenging to rethink something this complicated a year later so I will take some days to catch up to my older self, but an opportunity to make Zend actually work I do not want to miss. If this PR is closed, then several bug reports and PRs from others about sequences will have to be ignored: they are very deep rabbit holes.

@ezimuel It is a lot of code indeed :( For now, would you be able to at least go through conversations I had linked from #186? Especially the choice to be made in last comment of that issue, while keeping in mind last conversation at #162 where I mention that PDO driver solves some of the problems out of the box. (as an extra, may glance through #177)

Also, if I can assume #233 will be merged in, that will help, because it resolves quoting duplication logic.

[alextech]

@ezimuel Moving some of current questions I have from original bug report here so its easier for you to follow. Thank you for the blog post! Was motivational. Not pressuring you to answer these right away

Original code had switch/case statement per platform. Thats what bug reports focused on so I followed same pattern: https://github.com/zendframework/zend-db/pull/187/files#diff-85fe938f5a6f87443ad2578472eed9e0R166

But I just noticed there is duplication of that lastSequenceId() query in Connection class. Seems like a great benefit because do not need nasty switch($platform), and if anyone relied on similarly broken code in Connection too, all is fixed in one place.

https://github.com/zendframework/zend-db/blob/master/src/Adapter/Driver/Pgsql/Connection.php#L267

https://github.com/zendframework/zend-db/blob/master/src/Adapter/Driver/Pdo/Connection.php#L419

There are some concerns, however. What about nextSequenceId()? Connection interface does not have signature for it the way getLastGeneratedValue() is and rightfully so, I believe, should not be added. I could add nextSequenceId() to connection classes without adding to interface and assume dynamic typing will handle it, but that looks dirty. But code duplication looks dirty too, and so does inconsistency of doing switch/case for one, and call to adapter for the other method.

Another possible downside of using adapter connection, is getLastGeneratedValue() does not share same syntax for SqlServer 2016 where its one command for IDENTITY column but a different one for SEQUENCE object/column. (not a problem for postgresql or oracle up to 12c because sequence is the only way to do it)

Finally, how do I go about deducing implicit sequence names which too is a platform specific action? My getSequenceName() is awefully postgresql specific. I am not sure if decorator is possible to do for TableGateway features.

[weierophinney]

This repository has been moved to laminas/laminas-db. If you feel that this patch is still relevant, please re-open against that repository, and reference this issue. To re-open, we suggest the following workflow:

• Squash all commits in your branch (git rebase -i origin/{branch})
• Make a note of all changed files (`git diff --name-only origin/{branch}...HEAD
• Run the laminas/laminas-migration tool on the code.
• Clone laminas/laminas-db to another directory.
• Copy the files from the second bullet point to the clone of laminas/laminas-db.
• In your clone of laminas/laminas-db, commit the files, push to your fork, and open the new PR. We will be providing tooling via laminas/laminas-migration soon to help automate the process.

[michalbundyra]

This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at https://github.com/laminas/laminas-db/issues/88.