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

Allow arithmetic expressions within IN operator #6476

Closed
wants to merge 1 commit into from

Conversation

astepin
Copy link
Contributor

@astepin astepin commented May 28, 2017

In some cases it is necessary to use not only scalar values or subselects within IN-conditions, but also own functions.

With Oracle databases, the following is then possible:

DQL:
SELECT u FROM User WHERE u.name IN ( CUSTOM_DQL_FUNCTION() );

SQL:
SELECT * FROM user WHERE name IN ( SELECT * FROM TABLE(ORACLE_PLSQL_FUNCTION()) );

Another example which is also possible now:

DQL:
[...] WHERE field IN ( CUSTOM_DQL_GET_ID('Foo'), CUSTOM_ESCAPE('Bar') )

SQL:
[...] WHERE field IN ( (SELECT id FROM settings WHERE name = 'Foo'), [...] )

@Ocramius
Copy link
Member

Ocramius commented Jun 1, 2017

The idea sounds sane, but how does it behave with the "modify limit" logic in the paginator?

@astepin
Copy link
Contributor Author

astepin commented Jun 6, 2017

As far as i know, does the limit clause not affect any where conditions at all. I don't see any conflicts for the pagination.

@ostrolucky

This comment has been minimized.

@astepin

This comment has been minimized.

@Ocramius

This comment has been minimized.

@Ocramius Ocramius added this to the 3.0 milestone Oct 15, 2018
@astepin

This comment has been minimized.

ostrolucky
ostrolucky previously approved these changes Mar 20, 2019
SenseException
SenseException previously approved these changes Mar 20, 2019
@astepin astepin requested a review from Ocramius July 7, 2020 15:18
Base automatically changed from master to old-prototype-3.x February 23, 2021 08:18
@webda2l

This comment has been minimized.

@astepin

This comment has been minimized.

@greg0ire

This comment has been minimized.

@greg0ire greg0ire changed the base branch from old-prototype-3.x to 2.11.x October 5, 2021 17:44
@greg0ire greg0ire dismissed stale reviews from SenseException and ostrolucky October 5, 2021 17:44

The base branch was changed.

@derrabus

This comment has been minimized.

@derrabus derrabus modified the milestones: 3.0.0, 2.11.0 Oct 5, 2021
@greg0ire

This comment has been minimized.

@greg0ire

This comment has been minimized.

@@ -2759,7 +2759,7 @@ public function Literal()
}

/**
* InParameter ::= Literal | InputParameter
* InParameter ::= ArithmeticExpression | InputParameter
*
* @return AST\InputParameter|AST\Literal
Copy link
Member

Choose a reason for hiding this comment

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

As PHPStan pointed out, This line needs to be updated

Copy link
Contributor

@webda2l webda2l Oct 15, 2021

Choose a reason for hiding this comment

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

@astepin You asked how continue this PR #6476 (comment), there is an awaited commit fix here if you want.
See https://github.com/doctrine/orm/pull/6476/checks?check_run_id=3806327129#step:6:30.

Error: Method Doctrine\ORM\Query\Parser::InParameter() should return Doctrine\ORM\Query\AST\InputParameter|Doctrine\ORM\Query\AST\Literal but returns Doctrine\ORM\Query\AST\ArithmeticExpression.

so, @return AST\ArithmeticExpression

{
$this->entityManager->getConfiguration()->addCustomStringFunction('FOO', MyAbsFunction::class);
$this->assertSqlGeneration(
"SELECT u FROM Doctrine\Tests\Models\Forum\ForumUser u WHERE u.username IN (FOO('Lo'), 'Lo', :name)",
Copy link
Member

Choose a reason for hiding this comment

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

Why are you using strings in this example? It does not make a lot of sense to use an arithmetic function on a string, does it? Can you maybe write something closer to the real-world use-case you have?

Copy link
Member

Choose a reason for hiding this comment

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

You can disregard my comment if that's too hard: I just read the EBNF for https://www.doctrine-project.org/projects/doctrine-orm/en/2.9/reference/dql-doctrine-query-language.html#arithmetic-expressions and now I understand it's not just about 2+2 🙂

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In my use case, the scenario looks like this: I have a database platform specific function that I want to use like NVL2 from Oracle. For this I have created a user defined function for DQL in Doctrine and now I want to use this also for IN where conditions.

Copy link
Member

Choose a reason for hiding this comment

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

OK. To me it's kind of weird that ArithmeticPrimary contains that many things, for instance FunctionsReturningStrings. I don't think this has much to do with arithmetic, but anyway, that's not really your issue.

Copy link
Member

Choose a reason for hiding this comment

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

You could also add an arithmetic expression, it should not work before to do IN (1+1)

@derrabus derrabus removed the request for review from Ocramius October 5, 2021 18:17
@webda2l
Copy link
Contributor

webda2l commented Dec 9, 2021

@astepin so you have time to update this PR (see #6476 (comment)) or I do another PR ? Thanks

@astepin
Copy link
Contributor Author

astepin commented Dec 12, 2021

@astepin so you have time to update this PR (see #6476 (comment)) or I do another PR ? Thanks

Hey @webda2l, I will not be able to finish it this year. If you want to take it over, it's okay with me.

@derrabus
Copy link
Member

Closing in favor of #9242.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants