Skip to content

Simplify boolean filter#1905

Merged
dunglas merged 1 commit into
api-platform:2.2from
bendavies:simplify-boolean-filter
Apr 30, 2018
Merged

Simplify boolean filter#1905
dunglas merged 1 commit into
api-platform:2.2from
bendavies:simplify-boolean-filter

Conversation

@bendavies

Copy link
Copy Markdown
Contributor
Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? no
Tests pass? yes
Fixed tickets
License MIT
Doc PR

no need for extra query builder named parameters here. just use $expr->literal(...)

@bendavies bendavies changed the base branch from master to 2.2 April 30, 2018 09:37
@dunglas dunglas merged commit 7fb6254 into api-platform:2.2 Apr 30, 2018
@teohhanhui

Copy link
Copy Markdown
Contributor

This has implications for the DBMS prepared statement, no? I don't see why we'd want to do this, to be honest. Does it give a sizeable performance gain?

@bendavies

Copy link
Copy Markdown
Contributor Author

The reason i did this because

  1. there is no need for the parameter.
  2. the sql produced by symfonys profiler is invalid before this change, (at least for postgres - it produces where field = 1/0 which is wrong)

@teohhanhui

Copy link
Copy Markdown
Contributor

That's a Symfony profiler bug.

@soyuka

soyuka commented Apr 30, 2018

Copy link
Copy Markdown
Member

Isn't doctrine storing booleans as integers anyway?

@teohhanhui

teohhanhui commented Apr 30, 2018

Copy link
Copy Markdown
Contributor

Isn't doctrine storing booleans as integers anyway?

No. Not on PostgreSQL at least. The integers 0 and 1 are not valid boolean values: https://www.postgresql.org/docs/9.1/static/datatype-boolean.html

https://github.com/doctrine/dbal/blob/v2.7.1/lib/Doctrine/DBAL/Platforms/PostgreSqlPlatform.php#L66-L86

@bendavies

Copy link
Copy Markdown
Contributor Author

@teohhanhui

That's a Symfony profiler bug.

Well yes that's stating the obvious. I still don't understand the objection.

Cheers

@teohhanhui

Copy link
Copy Markdown
Contributor

I'm against it because a value is a value, not a literal. If there's a sizeable performance gain, I'm okay with making exceptions for the rule. But otherwise, I don't see why we'd do that.

teohhanhui pushed a commit to teohhanhui/api-platform-core that referenced this pull request May 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants