Fixed invalid quoting for postgresql adapter

[xorock]

As described in #161

[andrey-mokhov]

change to:

$sql = 'SELECT NEXTVAL(' . $platform->quoteValue($platform->quoteIdentifierChain($this->sequenceName)) . ')';

[andrey-mokhov]

change to:

$sql = 'SELECT CURRVAL(' . $platform->quoteValue($platform->quoteIdentifierChain($this->sequenceName)) . ')';

[alextech]

Just in case maintainers deem necessary to fix this before I am done with my overly prolonged enterprise of making sequences feature complete, I would suggest that at least this PR should be fixed in
https://github.com/zendframework/zend-db/blob/master/src/Adapter/Driver/Pgsql/Connection.php#L267
and sequence feature changed to

return $sequence['nextval'] = $this->tableGateway->adapter->driver->getLastGeneratedValue();

Two reasons:

1. By redirecting to Connection instead of writing out SQL for sequence value querying, if the developer is using PDO then this is not a problem because https://github.com/zendframework/zend-db/blob/master/src/Adapter/Driver/Pdo/Connection.php#L419 takes care of this more accurately at driver level. Also helps with other engines preventing an ugly switch/case block.
2. Code duplication resulting in chance that code from other places is relying on this causing errors.

If any queries are done manually they should be done using prepared statements to avoid overload on database engines to recreate execution plan for multitude of similar queries.

[ezimuel]

@alextech this PR can be closed in favor of your #187 ?

[alextech]

@ezimuel yes

[alextech]

@ezimuel also very closely related to #233 you asked about earlier.

[xorock]

Maybe instead of closing it You can simply accept it (for now)? It does not interfere with anything else and it's strange, that so simple one-liner was not reviewed and fixed for over a year. All signs in the sky and earth indicate that the next few months / years will pass before someone approves the @alextech fixes.

[alextech]

@xorock fair enough, if ever get back to my stuff I can always rebase (as part of relearning process I would have to undergo at that point anyway)

However!, this business of quoteIdentifierChain vs quoteIdentifier quoteValue that is pointed out in the review by andrey-mokhov must stop which I fix in #233. Maybe you can put a word in for that?

Edit--- n/m quoteValue is unrelated to identifier chain. I meant to say identifier vs identifierChain which I think fixes inconsistencies you reported when using schema names.

Edit2. I saw a nice blog post just coming out promising these new features, so lets just use that as motivation for focusing on solving the architecture difficulties :) zend team really is busy.

[weierophinney]

@xorock —

All signs in the sky and earth indicate that the next few months / years will pass before someone approves the @alextech fixes.

This sort of comment is not helpful, and disrespectful to the people maintaining the library. Please review our code of conduct, specifically points 5-7.

While I understand your frustration with a patch not being merged on a time schedule convenient to yourself, please be aware that we have around 200 components and modules to maintain, with only a couple dozen (mostly volunteer) people with commit rights. Please respect their efforts and time.

[xorock]

Sorry @weierophinney but I think disrespectful was when @alextech put a lot of effort into creating tests and fixing entire system and ezimuel just closed it: #187 (comment). I fully understand how much components You are maintaining, I'm with ZF from ~v0.7 but also I think zend-db is one of the most important elements of the entire framework and should be patched much faster. As Your team noticed https://framework.zend.com/blog/2017-12-06-zend-db-2.9.0.html?utm_source=dlvr.it&utm_medium=twitter "This is our first new feature release in over 18 months".
I don't do it because of my personal benefits (I fixed it in my code long time ago). We all do it so that the community can benefit.

[froschdesign]

@xorock

I think disrespectful was when @alextech put a lot of effort into creating tests and fixing entire system and ezimuel just closed it: #187

Yes this was a mistake in a hurry, but the PR is open and nothing is lost.

I think zend-db is one of the most important elements of the entire framework

Sorry, but that's only your feeling. Compare with the number of downloads on packagist.org: https://packagist.org/?q=zend

[xorock]

Funny thing is we're not discussing how to improve zend-db but other unimportant things. Truth is, both schemas and aliases (if I remember well Matthew noticed this bug ~3years ago, for example try to alias table and then DELETE something) needs to be rewritten. We should think how to do this.

As You know, packagist show statistics for all downloads. If You check requirements for zend-mvc, zend-form and zend-expressive You will know why these and not other packages are at the first place in statistics. zend-db is required manually.

[froschdesign]

@xorock

Funny thing is we're not discussing how to improve zend-db but other unimportant things.

It's really simple: with the new release zend-db is now in the focus. 😄

…needs to be rewritten. We should think how to do this.

Go for it! Slack and the forum is open for suggestions and discussions.

[michalbundyra]

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 laminas/laminas-db#98.