Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove broken assertion from DateAddFunction and DateSubFunction #11243

Merged
merged 2 commits into from
Feb 20, 2024
Merged

Remove broken assertion from DateAddFunction and DateSubFunction #11243

merged 2 commits into from
Feb 20, 2024

Conversation

ondrejmirtes
Copy link
Contributor

@ondrejmirtes ondrejmirtes commented Feb 9, 2024 •
edited
Loading

Closes #11240

@ondrejmirtes ondrejmirtes force-pushed the date_add_sub_non_numeric_sql branch 3 times, most recently from 5b8d2e7 to 2fc7661 Compare February 9, 2024 16:30
@ondrejmirtes
Copy link
Contributor Author

SA complains about DBAL 3.8.1 which accepts only int|numeric-string into these functions. In DBAL 4.0 string is accepted.

Should we ignore this here? Or should I send a PR to DBAL that widens the accepted type to int|string?

@ondrejmirtes ondrejmirtes mentioned this pull request Feb 9, 2024
Closed
@ondrejmirtes ondrejmirtes force-pushed the date_add_sub_non_numeric_sql branch from 2fc7661 to 1b5fc04 Compare February 9, 2024 16:32
@derrabus
Copy link
Member

derrabus commented Feb 9, 2024

Or should I send a PR to DBAL that widens the accepted type to int|string?

Yes please. And the deprecation that is triggered there for non-int values looks bogus, given that 4.0 still accepts arbitrary SQL expressions for seconds.

@ondrejmirtes ondrejmirtes mentioned this pull request Feb 9, 2024
Merged
@ondrejmirtes
Copy link
Contributor Author

Done: doctrine/dbal#6302

I didn't notice any wrong deprecations - all of them ask about is_int(...) which is fine - support for int is removed in 4.0.

derrabus pushed a commit to doctrine/dbal that referenced this pull request Feb 12, 2024
@ondrejmirtes
[3.8.x] AbstractPlatform - allow any string in interval for date inte…
a19a1d0 
…rval expression (#6302)

<!-- Fill in the relevant information below to help triage your pull
request. -->
|      Q       |   A
|------------- | -----------
| Type         | improvement

#### Summary

This change allows these methods to be called with just a `string` in
ORM 3.0.x. See
doctrine/orm#11243 (comment)

DBAL 4.0.x already has `string` there.

Also I'd like to note that I noticed that methods
`getDateAddQuartersExpression` and `getDateSubQuartersExpression` aren't
called from ORM. Shouldn't support for them be added?
@ondrejmirtes ondrejmirtes force-pushed the date_add_sub_non_numeric_sql branch from 1b5fc04 to 8b741d2 Compare February 15, 2024 07:29
@ondrejmirtes ondrejmirtes force-pushed the date_add_sub_non_numeric_sql branch from 8b741d2 to 687252b Compare February 15, 2024 07:30
@ondrejmirtes ondrejmirtes marked this pull request as ready for review February 15, 2024 07:34
@ondrejmirtes
Copy link
Contributor Author

This one is ready 😊

@derrabus derrabus added the Bug label Feb 15, 2024
@derrabus derrabus added this to the 3.0.1 milestone Feb 15, 2024
@derrabus derrabus changed the title Fix #11240 with assert(is_numeric()) Remove broken assertion from DateAddFunction and DateSubFunction Feb 20, 2024
@derrabus derrabus merged commit b6b4cbc into doctrine:3.0.x Feb 20, 2024
64 checks passed
@derrabus
Copy link
Member

Thank you!

derrabus added a commit to derrabus/orm that referenced this pull request Feb 21, 2024
@derrabus
Merge branch '3.0.x' into 3.1.x
fcf1116 
* 3.0.x:
  Remove broken assertion from DateAddFunction and DateSubFunction (doctrine#11243)
  Remove unused trait
  [Documentation] Adding link to Postgres upgrade article (doctrine#11257)
  fix: support array-type arg in QB variadic calls (doctrine#11242)
derrabus added a commit to derrabus/orm that referenced this pull request Feb 21, 2024
@derrabus
Merge branch '3.1.x' into 4.0.x
cbab4d6 
* 3.1.x:
  Fix Static Analysis folder reference (doctrine#11281)
  Remove broken assertion from DateAddFunction and DateSubFunction (doctrine#11243)
  Account for inversedBy being a non-falsy-string or null
  Improve static analysis on AttachEntityListenersListener
  docs: recommend safer way to disable logging (doctrine#11269)
  Remove unused baseline entries
  Treat '0' as a legitimate trim char
  Add type field mapper documentation to the sidebar
  Mark document as orphan
  Use correction sectionauthor syntax
  Remove unused trait
  [Documentation] Adding link to Postgres upgrade article (doctrine#11257)
  Validate more variadic parameters (doctrine#11261)
  Throw if a variadic parameter contains unexpected named arguments (doctrine#11260)
  Make docs valid according to guides 0.3.3 (doctrine#11252)
  fix: support array-type arg in QB variadic calls (doctrine#11242)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@derrabus derrabus derrabus approved these changes

Assignees
No one assigned
Labels
Bug
Projects
None yet
Milestone
3.0.1
Development

Successfully merging this pull request may close these issues.

Fix broken assert(is_numeric($sql)) in DateAddFunction and DateSubFunction
2 participants
@ondrejmirtes @derrabus