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.

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

Add support for more mathematical expressions #2791

Closed
wants to merge 2 commits into from
Closed

Add support for more mathematical expressions #2791

wants to merge 2 commits into from

Conversation

andreaskienast
Copy link

This patch adds support for these math expressions:

  • POWER
  • ASIN
  • TAN
  • ATAN
  • RADIANS
  • DEGREES

Fixes #2790

Andreas Fernandez added 2 commits July 26, 2017 09:57
This patch adds support for these math expressions:

* POWER
* ASIN
* TAN
* ATAN
* RADIANS
* DEGREES

Fixes #2790
@morozov
Copy link
Member

morozov commented Jul 26, 2017

@andreasfernandez, are implementations of these functions the same for all platforms (part of some SQL standard)? If so, why implement them in the abstraction layer which the DBAL is?

For some domains, both power() and log() may be used as inverse functions but we only support the former. We support pi() but don't support exp(). With this approach, we may end up porting everything to the DBAL.

If the team decides the DBAL still needs this, there also should be functional tests in addition to the unit ones to prove that the new code works correctly on all supported platforms.

@andreaskienast
Copy link
Author

@morozov, I have checked at least MySQL, MSSQL, Postgresql and Oracle, the functions are all named the same, but IMO that doesn't mean any other supported DMBS names them the same as well.

The fact that POWER is only an alias of POW in MySQL makes me feel unsure about using it as it may get dropped in future, which renders such a method necessary, IMO. The introduced method getPowerExpression() should be overridden in MySqlPlatform to use POW explicitly, then.

The original reason I created this patch is that I needed to write a DBAL-compliant radial search. I took this example by Google as template for the query and found more mathematical expressions that are missing.

@morozov
Copy link
Member

morozov commented Jul 26, 2017

The original reason I created this patch is that I needed to write a DBAL-compliant radial search. I took this example by Google as template for the query and found more mathematical expressions that are missing.

The DBAL allows using arbitrary SQL expressions in query parts, and in your example the resulting SQL doesn't depend on the platform. So the query like $query->select('id', '(3959 * ACOS(…) AS distance') is DBAL-compliant and portable, and doesn't require any modifications to the DBAL.

@morozov morozov closed this Oct 3, 2018
@morozov morozov self-assigned this Jun 7, 2019
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 7, 2022
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.

Support more mathematical expressions
2 participants