Skip to content

Reworked AbstractPlatform::get*Expression() methods#3498

Merged
Ocramius merged 1 commit intodoctrine:developfrom
morozov:get-other-expr
Mar 27, 2019
Merged

Reworked AbstractPlatform::get*Expression() methods#3498
Ocramius merged 1 commit intodoctrine:developfrom
morozov:get-other-expr

Conversation

@morozov
Copy link
Copy Markdown
Member

@morozov morozov commented Mar 27, 2019

Q A
Type improvement
BC Break yes

This pull request contains the breaking changes in AbstractPlatform::get*Expression() methods from #3348.

The key changes are:

  1. The arguments of the changed methods which previously could accept an SQL expression but weren't annotated so are now annotated.
  2. The arguments which were declared as int but de-facto accepted an SQL expression are now type-hinted as string.
  3. As a side effect of the above, with a slight rework in the implementation, the $interval argument of the getDate(Add|Sub)*Expression() methods can now accept an SQL expression which makes it possible to define the interval as a numeric literal (former behavior), prepared statement parameter, column name or technically any valid SQL expression 🔥
  4. The natively unsupported date intervals (e.g. weeks in SQLite) are now converted in SQL instead of the client side. Hence some changes in assertions against generated SQL on some platforms.
  5. DataAccessTest::testDateArithmetics() has been reworked into a series of independent tests each of which represents the interval in the query in three modes: numeric literal, prepared statement parameter and an SQL expression.
  6. As a side effect of the rework of SqlitePlatform::getDateArithmeticIntervalExpression(), it no longer truncates the time part of the result of addition or subtraction of days, weeks, months, quarters or years. This issue can be filed and fixed additionally in master since it impacts the code portability.
  7. The names of the arguments were renamed closer to their SQL analogs.
  8. The behavior implemented in Allow dynamic intervals in DATE_ADD & DATE_SUB for SQLite #3019 is now by design.

Copy link
Copy Markdown
Member

@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (besides nits)

@Ocramius Ocramius added this to the 3.0.0 milestone Mar 27, 2019
@Ocramius Ocramius merged commit 8cb2fb6 into doctrine:develop Mar 27, 2019
@Ocramius Ocramius assigned Ocramius and unassigned morozov Mar 27, 2019
@morozov morozov deleted the get-other-expr branch March 27, 2019 14:57
morozov pushed a commit that referenced this pull request Apr 16, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request May 6, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request May 23, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit to morozov/dbal that referenced this pull request May 31, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request Jun 13, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request Jun 27, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request Jun 27, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request Jun 27, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit to morozov/dbal that referenced this pull request Aug 26, 2019
Reworked AbstractPlatform::get*Expression() methods
morozov pushed a commit that referenced this pull request Nov 2, 2019
Reworked AbstractPlatform::get*Expression() methods
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants