5.x query abstract #16675
5.x query abstract #16675
Conversation
ADmad
force-pushed
the
5.x-query-abstract
branch
from
1eaacca
to
f9c9693
Compare
| public function newQuery(): Query | ||
| { | ||
| return new Query($this); | ||
| } |
There was a problem hiding this comment.
I wonder how big if a break this will be. There's nothing we could transform calls to newQuery() into.
There was a problem hiding this comment.
We could backport the newSelectQuery() methods to 4.x and add a deprecation to newQuery(). That would let us encourage users upgrade in a reasonable timeframe.
I'm happy to take care of that when this merges.
src/Database/IdentifierQuoter.php
Outdated
| } else { | ||
| $this->_quoteParts($query); | ||
| } elseif ($query instanceof DeleteQuery) { | ||
| $this->_quoteDelete($query); | ||
| } |
There was a problem hiding this comment.
should this have an else throw?
There was a problem hiding this comment.
Guess we could for completeness.
There was a problem hiding this comment.
Could get fancy and use match.
There was a problem hiding this comment.
Right, can use match(), still haven't got accustomed to the fact that it can use expressions for matching :).
| * @return void | ||
| */ | ||
| protected function _quoteParts(Query $query): void | ||
| protected function _quoteParts(Query $query, array $parts): void |
There was a problem hiding this comment.
Having the parts list separate from the Query instance doesn't feel right. When would we describe a query different than itself?
There was a problem hiding this comment.
These are just the part/clause names which need to be processed. They are different depending on the query type. The actual values for these parts are fetched from the Query itself.
| vendor/bin/phpunit tests/TestCase/Database --verbose --coverage-clover=coverage.xml | ||
| CAKE_TEST_AUTOQUOTE=1 vendor/bin/phpunit tests/TestCase/Database --verbose --testsuite=database |
There was a problem hiding this comment.
Do we need to revert this change before merging?
There was a problem hiding this comment.
Not yet, I will revert it when I getting to updating the ORM once the changes to database package are done.
src/Database/IdentifierQuoter.php
Outdated
| } else { | ||
| $this->_quoteParts($query); | ||
| } elseif ($query instanceof DeleteQuery) { | ||
| $this->_quoteDelete($query); | ||
| } |
There was a problem hiding this comment.
Could get fancy and use match.
|
Anymore feedback on this? |
Datbase\QueryabstractRefs #16648