Functional test for allowing dynamic intervals in date sub/add#3044
Functional test for allowing dynamic intervals in date sub/add#3044morozov merged 1 commit intodoctrine:masterfrom
Conversation
|
This is actually how the DBAL MySQL platform works - just making SQLite consistent as I use SQLite for functional tests and they're failing because of this inconsistency. |
|
@AshleyDawson could you include a failing test which is fixed by your changes in the pull request? |
|
Have written format tests for all unit/operator permutations when performing date add/subtract arithmetic for the SQLite platform |
|
@AshleyDawson I was talking about functional tests which failed for you and helped discover the issue. The ones which are executed against a real DB, not just make assertions against SQL. |
|
@morozov Sorry, should have explained in a previous comment. A query containing "DATE_SUB", etc. with a column reference instead of a literal fails silently and doesn't produce any results - with no exception raised. Only by inspecting the generated SQL can you see where the issue is. Please could you let me know where the SQLite functional tests for "DATE_SUB", etc. are and I'll modify them to take into account dynamic intervals. To explain the issue with examples: Current implementation, with literal (WORKS): ... WHERE DATE(r0_.next_renewal_at,'-30 DAY') ...Current implementation, with column reference (FAILS): ... WHERE DATE(r0_.next_renewal_at,'-r1_.renewal_risk_threshold_in_days DAY') ...(Notice the embedding of the column reference within the interval string) New implementation, with literal (WORKS): ... WHERE DATE(r0_.next_renewal_at,'-' || 30 || ' DAY') ...New implementation, with column reference (WORKS): ... WHERE DATE(r0_.next_renewal_at,'-' || r1_.renewal_risk_threshold_in_days || ' DAY') ... |
|
@AshleyDawson the logic you describe is the test I'm asking for.
This way, we'll validate the correctness of the SQL expression by means of a real DB and make sure this implementation survives any future rework without a regression. Furthermore, We'll make sure it works the same on other platforms which support that. |
|
@morozov I've now added a functional test as instructed to |
|
|
||
| public function testSqliteDateArithmeticWithDynamicInterval() | ||
| { | ||
| if (! $this->_conn->getDriver() instanceof PDOSqliteDriver) { |
There was a problem hiding this comment.
One last thing from me: please check platform instead of the driver since it's a platform-specific feature, not driver-specific.
There was a problem hiding this comment.
Good spot, I've made that change now.
|
@AshleyDawson looks good. Could you please rebase and squash all commits? The patch is quite straightforward and doesn't require all the commits to understand it. |
|
@morozov to be ported back to |
@Ocramius do we really need this, especially combined auto-dismissal of approvals after rebase? We don't usually have many concurrent conflicting changes. Porting to |
|
No, I think we're fine without that check. I will remove it when home.
…On 17 Mar 2018 19:49, "Sergei Morozov" ***@***.***> wrote:
This branch is out-of-date with the base branch
@Ocramius <https://github.com/ocramius> do we really need this,
especially combined auto-dismissal of approvals after rebase? We don't
usually have many concurrent conflicting changes.
Porting to 2.6 is fine with me.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3044 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAJakB6hUKeJc_RSOvi6S99khsvh1-pAks5tfVqmgaJpZM4So6Lw>
.
|
|
@AshleyDawson sorry for the delay. Could you please rebase? |
|
Looks like the issue is already resolved by #3019. I'll merge the functional test. |
Release v2.8.0 [](https://travis-ci.org/doctrine/dbal) This is a minor release of Doctrine DBAL that aggregates over 30 fixes and improvements developed over the last 3 months. This release includes all changes of the 2.7.x series, as well as feature additions and improvements that couldn’t land in patch releases. **Backwards Compatibility Breaks** This doesn't contain any intentional Backwards Compatibility (BC) breaks. **Dependency Changes** * The dependency on [doctrine/common](https://github.com/doctrine/common) is removed. DBAL now depends on [doctrine/cache](https://github.com/doctrine/cache) and [doctrine/event-manager](https://github.com/doctrine/event-manager) instead. Please see details in the [UPGRADE.md](UPGRADE.md) documentation. **Deprecations** * The usage of binary fields whose length exceeds the maximum field size on a given platform is deprecated. Please use binary fields of a size which fits all target platforms, or use blob explicitly instead. * The usage of DB-generated UUIDs is deprecated. Their format is inconsistent across supported platforms and therefore the feature is not portable. Use a PHP library (e.g. [ramsey/uuid](https://packagist.org/packages/ramsey/uuid)) to generate UUIDs on the application side. **New features** * Initial support of MySQL 8. * Initial support of PostgreSQL 11. * Ability to evaluate arbitrary SQL expressions via `AbstractPlatform::getDummySelectSQL()`. **Improvements and Fixes** * Improved support of binary fields on Oracle and IBM DB2. * Improved SQL Server configuration capabilities via both `sqlsrv` and `pdo_sqlsrv`. * Improved handling of `AUTOINCREMENT`ed primary keys in SQLite. * Integration tests are run against IBM DB2 on Travis CI. * Code coverage is collected for the Oracle platform on continuousphp. Total issues resolved: **33** **Deprecations:** - [3187: Deprecate usage of binary fields whose length exceeds maximum](doctrine#3187) thanks to @morozov - [3188: Deprecated usage of binary fields whose length exceeds the platform maximum](doctrine#3188) thanks to @morozov - [3192: Added more information to the deprecation notice](doctrine#3192) thanks to @morozov - [3212: Deprecated usage of DB-generated UUIDs](doctrine#3212) thanks to @morozov **New Features:** **Bug Fixes:** - [3149: Introduced binary binding type to support binary parameters on Oracle](doctrine#3149) thanks to @morozov - [3178: Fix incorrect exception thrown from SQLAnywhere16Platform](doctrine#3178) thanks to @Majkl578 - [3044: Functional test for allowing dynamic intervals in date sub/add](doctrine#3044) thanks to @AshleyDawson - [3049: Test failures caused by invalid database connection result in fatal error](doctrine#3049) thanks to @Majkl578 **Improvements:** - [3033: Added support for available DSN parameters for the PDOSqlsrv driver](doctrine#3033) thanks to @aashmelev - [3128: Add MySQL 8 reserved keywords](doctrine#3128) thanks to @mlocati - [3143: initialize sql array into platform files](doctrine#3143) thanks to @AlessandroMinoccheri - [3173: Fix composer branch aliases](doctrine#3173) thanks to @Majkl578 - [3157: When building a limit query, zero offset without a limit should be ignored](doctrine#3157) thanks to @morozov - [3109: Allow to specify arbitrary SQL expression in AbstractPlatform::getDummySelectSQL()](doctrine#3109) thanks to @morozov - [3141: allow creating PRIMARY KEY AUTOINCREMENT fields for sqlite (unit tests)](doctrine#3141) thanks to @TimoBakx - [3180: Import simplified version of Common\Util\Debug for var dumping purposes](doctrine#3180) thanks to @Majkl578 **Documentation Improvements:** - [3117: Added badges for the develop branch in README](doctrine#3117) thanks to @morozov - [3125: Upgrading docs](doctrine#3125) thanks to @jwage - [3144: added improvement type into pull request template](doctrine#3144) thanks to @AlessandroMinoccheri **Code Quality Improvements:** - [3025: Added PHPStan, apply changes for level 3](doctrine#3025) thanks to @Majkl578 - [3200: Php Inspections (EA Ultimate): minor code tweaks](doctrine#3200) thanks to @kalessil - [3204: Fix typo in AbstractPlatform](doctrine#3204) thanks to @Majkl578 - [3205: Ignore OCI-* classes in static analysis (no stubs)](doctrine#3205) thanks to @Majkl578 **Continuous Integration Improvements:** - [3102: Use newer PHPUnit to prevent crashes on failures](doctrine#3102) thanks to @Majkl578 - [3112: Removed hard-coded configuration filenames from the test runner](doctrine#3112) thanks to @morozov - [3133: Travis DB2](doctrine#3133) thanks to @Majkl578, @morozov - [3135: AppVeyor tweaks, retry coverage upload on failure](doctrine#3135) thanks to @Majkl578 - [3137: Workaround for the inability to use a post-PHPUnit script on ContinuousPHP](doctrine#3137) thanks to @morozov - [3151: MSSQL DLL 5.2.0 has been released.](doctrine#3151) thanks to @photodude - [3160: Test against Postgres 11](doctrine#3160) thanks to @Majkl578 **Dependencies** - [3193: DBAL 2.8 needs Common 2.9](doctrine#3193) thanks to @Majkl578 - [3176: Eliminate dependency on doctrine/common](doctrine#3176) thanks to @Majkl578 - [3181: Remove dependency on doctrine/common](doctrine#3181) thanks to @Majkl578 # gpg: Signature made Fri Jul 13 06:02:10 2018 # gpg: using RSA key 374EADAF543AE995 # gpg: Can't check signature: public key not found # Conflicts: # .gitignore # README.md # lib/Doctrine/DBAL/Version.php # tests/appveyor/mssql.sql2008r2sp2.sqlsrv.appveyor.xml # tests/appveyor/mssql.sql2012sp1.sqlsrv.appveyor.xml # tests/appveyor/mssql.sql2017.pdo_sqlsrv.appveyor.xml # tests/appveyor/mssql.sql2017.sqlsrv.appveyor.xml # tests/travis/mariadb.mysqli.travis.xml
Use catenation strategy to include either a literal interval or a dynamic interval from the database its self using a column reference