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

[8.x] Add better bitwise operators support #40529

Merged
merged 2 commits into from
Jan 22, 2022
Merged

[8.x] Add better bitwise operators support #40529

merged 2 commits into from
Jan 22, 2022

Conversation

Arzaroth
Copy link
Contributor

Impetus

This addresses the problem mentioned in issue #40484

Changes

Since the bitwise operators are fundamentally different from other, in the sense that they do not produce boolean expression but integer based expressions, I elected to treat them differently from others.

Currently, both MySQL and SQLite support "implicit casting" of integer to boolean, but SQL Server and PostgreSQL do not.
The idea is to have a uniform behavior across all supported databases.

Previously this would yield a PDOException on PostgreSQL for instance :

$builder->select('*')->from('foo')->where('bar', '&', 1)->get(); // MySQL is cool with that, PostgreSQL looks upon you and declares that argument of WHERE must be type boolean, not type integer

The proposed implementation is database agnostic (as far as I'm aware at least). This is done by forcing a comparison, thus transforming the expression into a boolean one.

What next

This could open the door to support for basic arithmetic operators (e.g. +, -, etc...), although I would argue those appear even more elusively in WHERE clauses.

Comment on lines +855 to +856
return in_array(strtolower($operator), $this->binaryOperators, true) ||
in_array(strtolower($operator), $this->grammar->getBinaryOperators(), true);
Copy link
Contributor

@Jubeki Jubeki Jan 21, 2022

Choose a reason for hiding this comment

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

What is your reason for calling strtolower($operator)? In my opinion the operator has no lower or upper case.

Suggested change
return in_array(strtolower($operator), $this->binaryOperators, true) ||
in_array(strtolower($operator), $this->grammar->getBinaryOperators(), true);
return in_array($operator, $this->binaryOperators, true) ||
in_array($operator, $this->grammar->getBinaryOperators(), true);

Copy link
Contributor Author

Choose a reason for hiding this comment

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

None in particular, I copied what I saw in the invalidOperator.

@nunomaduro nunomaduro changed the title Add better bitwise operators support [8.x] Add better bitwise operators support Jan 21, 2022
@taylorotwell taylorotwell merged commit 676954b into laravel:8.x Jan 22, 2022
@Arzaroth Arzaroth deleted the bitwise-operators branch January 22, 2022 23:22
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