[5.1] Update hydrate to handle binary stream resources #10857
4 participants
GrahamCampbell
commented on an outdated diff
| @@ -510,6 +510,20 @@ public static function hydrate(array $items, $connection = null) | ||
| $instance = (new static)->setConnection($connection); | ||
| $items = array_map(function ($item) use ($instance) { | ||
| + | ||
| + // This loop unwraps content in stream resource objects. | ||
| + // PostgreSQL will return binary data as a stream, which does not | ||
| + // cache correctly. Doing this allows proper attribute mutation and | ||
| + // type casting without any headache or checking which database | ||
| + // system we are using before doing business logic. | ||
| + // | ||
| + // See: https://github.com/laravel/framework/issues/10847 |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
GrahamCampbell
commented on an outdated diff
| @@ -510,6 +510,20 @@ public static function hydrate(array $items, $connection = null) | ||
| $instance = (new static)->setConnection($connection); | ||
| $items = array_map(function ($item) use ($instance) { | ||
| + | ||
| + // This loop unwraps content in stream resource objects. | ||
| + // PostgreSQL will return binary data as a stream, which does not | ||
| + // cache correctly. Doing this allows proper attribute mutation and | ||
| + // type casting without any headache or checking which database | ||
| + // system we are using before doing business logic. | ||
| + // |
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
|
GrahamCampbell
changed the title from Update hydrate to handle binary stream resources. to [5.1] Update hydrate to handle binary stream resources
@crynobone No, I don't think so. Review my ticket and the trouble I'm having with PostgreSQL and binary.
To get binary working with PostgreSQL and Laravel I have to ...
- Write helper methods to unwrap and escape values. See helpers.php
- Write mutators to read and store information correctly. See BoardSetting.php
- (This is the big one) Ensure that the
Eloquent->attributes[]value IS NOT astream resourceor theCachefacade will cache it as0and not the value of its stream. See EloquentBinary.php, my hack
The easiest, Occam's razor approach is what I've done. This simply unwraps the data from the stream so that the cache driver works without any problems, but forces the user to make sure that the data in the attributes array is what should be sent to the database using Eloquent mutators.
The best possible fix for this would be to have a database driver method that can be used to wrap and unwrap information, possibly using type casting. The problem with type casting is that it's basically just a streamlined attribute mutator that doesn't stop the caching issue. The difference between binary for MySQL and binary for PostgreSQL is a great deal (because you have to wrap/unwrap and unstream as I described). However, I don't see a way for that to be done with existing architecture.
I didn't want to go ahead and rewrite your PostgreSQL driver code for fear of wasting a lot of time I don't have to give only to have it rejected. I could do that later, but for right now, what I need is in that trait I linked. This code would help a lot of PostgreSQL people in the mean time.
I really hope all this makes sense. It's taken me a week to get this far.
To elaborate, you would send ip 192.168.1.10 to MySQL (and probably all other drivers) as this binary string using inet_pton:
b"ˬ\x01\n"
This, inserted into PostgreSQL, causes issues. It throws an error. To get it to work, you must use pg_encode_bytea. The output for that same string is:
\300\250\001\012
THIS is what must be inserted into PostgreSQL. However, on select, it returns a bizarre stream resource. To get an actual value, you must use stream_get_contents (my fix). Now you'll be back to the encoded string
\300\250\001\012
This is unusable, obviously, so we must also pg_unencode_bytea it.
b"ˬ\x01\n"
Now that we're back to binary, we can use inet_ntop to get 192.168.1.10 again. The issue is that ONLY PGSQL stores Binary like this.
My fix doesn't deal with most of that, it only deals with making sure that we never have a stream resource value in Eloquent, because that breaks the cache driver and there is no other way to handle it other than interrupting hydrate to make sure that Eloquent never sees that value.
Seems like you could use a mutator and be more efficient here and achieve the same purpose. We don't want to spin through every attribute of every result looking for resource types.
@taylorotwell No mate, read what I wrote. The Cache object does not call mutators. You have to either update your Cache manager to unpack streams, or improve your Database interfaces so they unpack streams (which is the best option).
Because the stream resource will cache as the integer value 0 for some reason, no matter what the actual binary data is. And, since the Cache tool doesn't call mutators, there's nothing I can do at a mutator level to fix it. I have to change hydrate() to unpack resources.
I really tried everything to avoid changing framework functions, but binary values collected from PostgreSQL are completely worthless in Laravel. They have to be inserted with pg_encode_bytea, selected and unpacked with stream_get_contents, and then finally decoded with pg_decode_bytea. This would all be much better done on a Framework level, but even with mutators doing some heavy lifting, you have to make absolutely sure that stream_get_contents is ran against the raw ->attributes items before any caching is done or it breaks.
@taylorotwell See: #10847 (comment)
I wrote out the entire exploration of this issue.
Eloquent->hydratenow runsstream_get_contentson resource columns before passing them into the model.This resolves an issue where it is impossible to adequately deal with binary data from PostgreSQL.
#10847