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

JSON in Databases can't be used because Eloquent doesn't support the data types #13232

Closed
sebwas opened this issue Apr 20, 2016 · 14 comments
Closed

Comments

@sebwas
Copy link
Contributor

sebwas commented Apr 20, 2016

Supposed you have a column attr in your table. Supposed your table has these entries:

+------+--------------------------------------+
| name | attr                                 |
+------+--------------------------------------+
| a    | {"a": 1, "b": 3.1415926, "c": "foo"} |
| b    | {"a": 2, "b": 1.7182818, "c": "bar"} |
+------+--------------------------------------+

Now using Eloquent to get the row where the value of a is 1, you would have use DB::raw(1) as the value in the call to where like this:

$model->where('attr->b', 1.7182818); // Does not work!
$model->where('attr->b', DB::raw(1.7182818)); // Does work, but isn't pretty and not "the Eloquent way"
@themsaid
Copy link
Member

themsaid commented Apr 20, 2016

This issue is handled in 5.3 by using the correct PDO parameter type, I didn't submit the fix to 5.2 because I thought it might be breaking, it's not breaking for MySQL and Postgres but I'm not sure about other engines.

@sebwas
Copy link
Contributor Author

sebwas commented Apr 20, 2016

That is not true. It does work for integers, right, but it does not for boolean or floating point numbers. So the issue is indeed still not resolved.

To check it out yourself, do the following:

Make a new laravel app, then use php artisan tinker to do the following:

// Create the table
Schema::create('tests', function($table){
    $table->string('name');
    $table->json('attr');
});
// Insert the test data
DB::table('tests')->insert([
    ['name' => 'a', 'attr' => json_encode(['a' => 1, 'b' => 3.141592654, 'c' => 'foo'])],
    ['name' => 'b', 'attr' => json_encode(['a' => 2, 'b' => 1.718281828, 'c' => 'bar'])],
]);
// Test it
DB::table('tests')->select('name', 'attr')->where('attr->b', 1.718281828)->get();

Instead of giving back the row with the name of b, this simply returns an empty collection:

Illuminate\Support\Collection {#658
    all: [],
}

See here for I tried it myself:
image

@themsaid
Copy link
Member

A patch was added to handle floats, however for booleans there's something with the PDO I agree but quoting Taylor here #13166 (comment)

Making our own PDO isn't on the table, heh. Any improvements should come on PDO::bindParam on the 5.3 / master branch.

I think we'll need to handle the matter using the PDO somehow, or just settle for strings and integers and hint that in the docs for now.

@sebwas
Copy link
Contributor Author

sebwas commented Apr 20, 2016

[...] or just settle for strings and integers and hint that in the docs for now.

Just ignoring the issue is not gonna make it go away.

It is in fact not doable with the PDO, so why wouldn't we fix it? I see we can't build our own PDO. So why not accept the fix in #13166 or come up with another solution?
Just settling at that unfinished point is not what Laravel is about at all as far as I know.

@sebwas
Copy link
Contributor Author

sebwas commented Apr 20, 2016

@GrahamCampbell please reopen this issue as nothing has been resolved here, while the issue is a real issue

@sebwas
Copy link
Contributor Author

sebwas commented Apr 20, 2016

@taylorotwell I'd like to hear your opinion about that.

@GrahamCampbell
Copy link
Member

GrahamCampbell commented Apr 20, 2016

@themsaid What do you make of this please?

@themsaid
Copy link
Member

+------+--------------------------------------+
| name | attr                                 |
+------+--------------------------------------+
| a    | {"a": true}                          |
+------+--------------------------------------+
DB::table('table')->where('attr->a', true)->first()

It'll return null while it should return the existing record, PDO::PARAM_BOOL doesn't work because of a known issue https://bugs.php.net/bug.php?id=66632 with MySQL as it doesn't support bools so it's considered a string, however MySQL JSON has a bool type and expects a bool value.

In my opinion the only way we can deal with this is by manually injecting true|false into the query instead of using PDO param in case of a JSON query. If this fix isn't acceptable then we should note in the docs that bools should be passed by DB::raw() in case of querying JSON.

DB::table('table')->where('attr->a', DB::raw('true'))->first()

Regarding doubles, it's handled in 5.3 now. So strings, integers, and doubles are ok, bools no.

@sebwas
Copy link
Contributor Author

sebwas commented Apr 20, 2016

I kinda got the vibe that you guys don't give anything on my opinion, but still this is not where it should end.

Why not let the fix from #13166 do the bool work for you instead of having to manually use DB::raw all the time, despite it being very counter-intuitive.

@themsaid
Copy link
Member

@sebwas I'm saying that your fix for bools is the only way in my opinion, for doubles it can be handled by PDO::PARAM_INT but for bools, your fix is the only possible solution AFAIK.

@jaketoolson
Copy link

@sebwas
This isn't an issue with Laravel. It's a PDO issue when binding and preparing statements.

@sebwas
Copy link
Contributor Author

sebwas commented Apr 20, 2016

@jaketoolson I'm perfectly aware of that. The thing is that you can get around that, which is where laravel comes in.

@billmn
Copy link
Contributor

billmn commented Jun 3, 2016

@sebwas This issue can be closed?

@taylorotwell
Copy link
Member

Yes, was fixed.

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

No branches or pull requests

6 participants