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

.filter(.field == nil) not working anymore #120

Closed
proggeramlug opened this Issue Jun 20, 2018 · 6 comments

Comments

2 participants
@proggeramlug
Copy link

proggeramlug commented Jun 20, 2018

When using something like:

.filter(\field == nil)

the MySQL Output becomes "field == null", which really needs to be "field IS NULL".

@proggeramlug

This comment has been minimized.

Copy link
Author

proggeramlug commented Jun 20, 2018

Fixed here: vapor/sql#14

@tanner0101

This comment has been minimized.

Copy link
Member

tanner0101 commented Jun 21, 2018

@proggeramlug are you sure that PR fixed the issue? That == operator is not the one used in .filter(<keyPath> == <value>). This is the operator used, and it already includes that check.

@proggeramlug

This comment has been minimized.

Copy link
Author

proggeramlug commented Jun 21, 2018

@tanner0101 Try it yourself, the one you referenced (https://github.com/vapor/fluent/blob/master/Sources/Fluent/QueryBuilder/QueryBuilder%2BOperators.swift#L142) is not working for me, like it's not the one used here.

@proggeramlug

This comment has been minimized.

Copy link
Author

proggeramlug commented Jun 21, 2018

Also checkout this one, where the same problem is observed: vapor/sql#13

@proggeramlug

This comment has been minimized.

Copy link
Author

proggeramlug commented Jun 21, 2018

Okay, I just created a test project that illustrates the case: https://github.com/proggeramlug/vapor-niltest

I also have a big hint for the fix (if you have no time to do this any time soon let me know and I'll gladly dig deeper).

So in the TodoController.swift is an import Fluent statement as well as an import FluentMySQL statement. If you remove the import FluentMySQL statement it works as expected:

$ vapor build --run --verbose
Building Project ...
Building Project [Done]
Running NilTest ...
[ INFO ] Migrating 'mysql' database (MigrationConfig.swift:69)
[mysql] [2018-06-21 21:09:36 +0000] SELECT COUNT(*) AS `fluentAggregate` FROM `fluent` []
[mysql] [2018-06-21 21:09:36 +0000] SELECT * FROM `fluent` ORDER BY `fluent`.`batch` DESC LIMIT 1 OFFSET 0 []
[mysql] [2018-06-21 21:09:36 +0000] SELECT * FROM `fluent` WHERE `fluent`.`name` = (?) LIMIT 1 OFFSET 0 [string("Todo")]
[ INFO ] Migrations complete (MigrationConfig.swift:73)
Running default command: .build/debug/Run serve
Server starting on http://localhost:8080
[mysql] [2018-06-21 21:09:46 +0000] SELECT * FROM `Todo` WHERE `Todo`.`id` = (?) AND `Todo`.`testField` IS NULL [integer8(1)]

However, if you leave the FluentMySQL in there it will be wrong:

$ vapor build --run --verbose
Building Project ...
Compile Swift Module 'App' (6 sources)
Linking ./.build/x86_64-apple-macosx10.10/debug/Run
Building Project [Done]
Running NilTest ...
[ INFO ] Migrating 'mysql' database (MigrationConfig.swift:69)
[mysql] [2018-06-21 21:11:09 +0000] SELECT COUNT(*) AS `fluentAggregate` FROM `fluent` []
[mysql] [2018-06-21 21:11:10 +0000] SELECT * FROM `fluent` ORDER BY `fluent`.`batch` DESC LIMIT 1 OFFSET 0 []
[mysql] [2018-06-21 21:11:10 +0000] SELECT * FROM `fluent` WHERE `fluent`.`name` = (?) LIMIT 1 OFFSET 0 [string("Todo")]
[ INFO ] Migrations complete (MigrationConfig.swift:73)
Running default command: .build/debug/Run serve
Server starting on http://localhost:8080
[mysql] [2018-06-21 21:11:12 +0000] SELECT * FROM `Todo` WHERE `Todo`.`id` = ? AND `Todo`.`testField` = ? [integer8(1), null]

So it is somehow tied to the FluentMySQL dependency. My PR mentioned above fixes it but the problem may be way before that one even kicks in.

@tanner0101

This comment has been minimized.

Copy link
Member

tanner0101 commented Jun 21, 2018

Oh, I see now. Great find. There is an overload on filter(...) that accepts the DB's generic filter type. For MySQL, that is MySQLExpression. Since MySQL also offers an == that generates a MySQLExpression, that one is being chosen instead.

Very interesting problem. I think that the filter overload that accepts the raw filter type should probably be prefixed with raw: to be clear.

Like:

filter(raw: \.foo == bar)

That way there's no ambiguity. With that said, you are correct and vapor/sql#13 should be merged :)

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