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

Fix: Problem to use partial searchFilter with an integer as input. #4356

Closed
wants to merge 1 commit into from

Conversation

hogren
Copy link

@hogren hogren commented Jul 13, 2021

SQL Server does not automatically cast an integer to a string in
the concatenation with + method (...WHERE field1 LIKE :param1 + :param2)

So there is an SQL Server error when we try to enter "1" in a partial filter :
An exception occurred while executing 'SELECT […] WHERE b0_.label LIKE ('%' + ? + '%') ORDER BY […]' with params [1]:\n\nSQLSTATE [22018, 245]: [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Conversion failed when converting the varchar value '%' to data type int.

The CONCAT function is not available before the 2012 version.

I so simply force the casting when the searchFilter is not of type exact.

Q A
Branch? current
Tickets not declared
License MIT

About tests, I did not find your test suite. So don’t hesitate to give me the right procedure.

EDIT : After Alan Poulain message, I run tests without any problem.

@alanpoulain
Copy link
Member

SQL Server does not automatically cast an integer to a string in
the concatenation with + method (...WHERE field1 LIKE :param1 + :param2)
The CONCAT function is not available before the 2012 version.

I so simply force the casting when the searchFilter is not of type exact.
@hogren hogren changed the title Problem to use partial searchFilter with an integer as input. Fix: Problem to use partial searchFilter with an integer as input. Jul 17, 2021
@hogren
Copy link
Author

hogren commented Jul 18, 2021

@alanpoulain Thanks . No problem with tests :

$ vendor/bin/simple-phpunit --stop-on-failure -vvv tests/Bridge/Doctrine/Orm/Filter/SearchFilterTest.php
PHPUnit 9.4.4 by Sebastian Bergmann and contributors.

Runtime: PHP 8.0.6
Configuration: …\vendor\api-platform\core\phpunit.xml.dist

Testing ApiPlatform\Core\Tests\Bridge\Doctrine\Orm\Filter\SearchFilterTest
................................................................. 65 / 71 ( 91%)
...... 71 / 71 (100%)

Time: 00:11.159, Memory: 78.00 MB

OK (71 tests, 211 assertions)

Legacy deprecation notices (102)

@hogren
Copy link
Author

hogren commented Jul 26, 2021

@alanpoulain : Hello, just to be sure. Is everything ok for my PR or is there anything I forgot to do ?
Thanks !

Regards,

@soyuka
Copy link
Member

soyuka commented Aug 10, 2021

The CONCAT function is not available before the 2012 version.

I do think that you should implement your own filter as we don't want to support older version no?

@hogren
Copy link
Author

hogren commented Aug 11, 2021

@soyuka Sorry, this info was confusing. We have recent version, it’s not the problem.
In fact, the Doctrine Concat use the "+" method (maybe because the concat is not available in old versions). But it add quotes only if the php variable is a string.
With my correction, all concerned php variables are be casting in string.

@hogren
Copy link
Author

hogren commented Nov 11, 2021

@alanpoulain , @soyuka Hello !
Do you have thought about my PR ?
Can I make any improvement ? Or do you need any additional information ?

Thank you !

@alessandrofilira
Copy link

@soyuka Sorry, this info was confusing. We have recent version, it’s not the problem. In fact, the Doctrine Concat use the "+" method (maybe because the concat is not available in old versions). But it add quotes only if the php variable is a string. With my correction, all concerned php variables are be casting in string.

Hi @hogren , I used the following workaround: I set filter strategy to end and when I call the API I append % in the end of value so doctrine convert it in string. So the call is api/entity?code=34%.

It's not really good solution but it works!

@hogren
Copy link
Author

hogren commented Sep 7, 2022

@alessandrofilira Hello, sorry for the long time of silence. I have never seen your answer.
Thank you very much for the workaround, but the goal of my PR is to fix the problem. I already have another workaround.

@stale
Copy link

stale bot commented Nov 6, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Nov 6, 2022
@stale
Copy link

stale bot commented Jan 5, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Jan 5, 2023
@stale stale bot closed this Jan 12, 2023
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.

4 participants