#186 [RFC][WIP] Sequence Support #187
Conversation
…yle columns in PostgreSQL
Amendment to how SequenceFeatures are filtered from FeatureSet to match documentation. Since sequence names are not always known + more natural to interact with TableGateway using column names, filter function call by column names. Open to suggestion on how to improve this and filter somehow from FeatureSet *before* attempt to execute method on the feature (challenge to do so that does not break other feature types). Also, since SequenceFeature is more involved and requires long description, can consider moving to own docs page.
Docs SQL alignment
…er than 63 bytes for postgresql, query the actual name.
…quence does not imply primary key.
…ncrement interval.
… to specify schema.
…tform and adapter objects
|
Unfortunately, I've to close this PR due to inactivity. |
|
@ezimuel I was waiting on some assistance about difficult architecture decisions outlined in last comment at #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 :( |
|
@alextech ok, I'm re-opening this PR. I'll try to schedule some time for review it. It's a lot of code :( |
|
@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. |
|
@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. |
|
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:
|
|
This repository has been closed and moved to laminas/laminas-db; a new issue has been opened at laminas/laminas-db#88. |
For original thought process on this list see #186
SERIALpseudo-typeSERIALpseudo-type to column creation DDL to compliment idea of implicit sequence at DB creation/migrations level.InsertandUpdatestatements to filter out or generate values based on the changesSequenceFeatureindependent fromTableGatewaysince (1) it does not necesseirly tie to a specific table, (2) fixRowGatewaylimitationUpdate 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:
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.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.