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

Eloquent unable to save Postgres arrays #27616

Closed
gareth-ib opened this issue Feb 21, 2019 · 25 comments
Closed

Eloquent unable to save Postgres arrays #27616

gareth-ib opened this issue Feb 21, 2019 · 25 comments

Comments

@gareth-ib
Copy link

  • Laravel Version: v5.7.27
  • PHP Version: 7.2.2
  • Database Driver & Version: PostgreSQL 10.5

Description:

The eloquent code is treating a database array and a json object as if they are the same. But in PostGres the format for an array like an INTEGER[] needs formatting to be as

INSERT INTO schema.table ( field ) ( '{1,2,3}' );

but because of current setup using json_encode, the result is

INSERT INTO schema.table ( field ) ( '[1,2,3]' );

The minor but crucial difference being the [ and the {

Steps To Reproduce:

CREATE TABLE testschema.testtable (
  arrayfield integer[]
);

with Model cast defining array:

use Illuminate\Database\Eloquent\Model;
class TestTable extends Model {
	protected $casts = [
		'arrayfield ' => 'array',
	];
}
\App\Models\Incoming\TestSchema\Placement::updateOrCreate( [
 'arrayfield' => [1,2,3]
] );

will generate...

insert into schema.testtable ( arrayfield ) values ([11])

which gives the error

Invalid text representation: 7 ERROR:  malformed array literal: "[11]"
@driesvints
Copy link
Member

I'll need someone to verify this on PostGres.

@staudenmeir
Copy link
Contributor

This is not supported. Eloquent can't know whether you want to store a PostgreSQL array or a JSON string.

We could stop having the array cast as an alias for json and only use it for PostgreSQL arrays, but that would be a huge breaking change.

You'll have to add a custom accessor and mutator:

class Test extends Model {

    public function getFieldAttribute($value) {
        return json_decode(str_replace(['{', '}'], ['[', ']'], $value));
    }

    public function setFieldAttribute($value) {
        $this->attributes['field'] = str_replace(['[', ']'], ['{', '}'], json_encode($value));
    }

}

@gareth-ib
Copy link
Author

JSON and ARRAY are specifically separate things. So yes the code that treats them the same never was correct. It was a never tested always broken scenario. The json_encode method cannot be used at all for a Postgres ARRAY.
Array's can also be strings, example: CHARACTER VARYING[] which can contain those [ ] { } characters. It's not just for INTEGER[].
And Eloquent specifically already knows by the $cast whether that field is an ARRAY or JSON.

So the lines like this...

            case 'array':
            case 'json':
                return $this->fromJson($value);

from the castAttribute method in laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php

which ignored the differences between those two data types, are the issue.

@staudenmeir
Copy link
Contributor

My example is meant for the integer arrays from your example.

You can also try this package: https://github.com/asmiarowski/laravel-postgres

Array columns are only supported by PostgreSQL and not very widely used (my impression). So they simply weren't considered when that code was written. Not so simple to change it now.

@gareth-ib
Copy link
Author

That kinda supports my point ( I think ). What exactly are Array columns outside of PGSQL? MySQL doesn't have them. So is this problem code even executing in other scenarios?

@staudenmeir
Copy link
Contributor

How do you mean? The array cast is the default way to store values as JSON.

@gareth-ib
Copy link
Author

oh cuz cast is used in a fake way with text fields... got it...
so this fake array casting concept is breaking the real array db functionality...

@gareth-ib
Copy link
Author

is there another attribute system other than cast that holds the actual data type of the field? Or is cast the only system... which apparently contains real and faked concepts

@staudenmeir
Copy link
Contributor

It's not a "fake way", MySQL and PostgreSQL do have native JSON column types.

@gareth-ib
Copy link
Author

I'm not talking about JSON at all though, I'm talking about ARRAY

@gareth-ib
Copy link
Author

so this fake array casting functionality makes it so that generated models for postgres array fields are broken... hence my issue now

@staudenmeir
Copy link
Contributor

JSON is a reasonable way to store arrays, especially when it's supported by all databases.

It doesn't break PostgreSQL arrays, they wouldn't work natively either way.

@gareth-ib
Copy link
Author

ok but still this is not a conversation about json, it's about arrays. Not the Laravel specific idea to store json into a text field and call it array. And an array field is not in json format. Which is why this faked feature is breaking arrays.

@gareth-ib
Copy link
Author

information on postgres database arrays. PGSQL docs
which, if you are only familiar with MySQL functionality, you'll not already understand

@staudenmeir
Copy link
Contributor

staudenmeir commented Feb 22, 2019

I'm familiar with PostgreSQL arrays.

The point of the array cast is that the user shouldn't have to worry about the internal implementation. The user sets and gets a PHP array, while Laravel handles the serialization. JSON is currently the best way to achieve that.

I assume that the documentation suggests to use array (instead of json) because that's what the cast does, not how it does that.

@driesvints
Copy link
Member

Since it seems that this isn't really a bug but more of a request for more native Postgres support, I think it's best that this is moved to the ideas repo.

@gareth-ib
Copy link
Author

eloquent already supports postgres, but this feature was built and breaks a postgres feature that already existed before eloquent was made.

@mfn
Copy link
Contributor

mfn commented Feb 22, 2019

To me it seems indeed that:

  • json should be used whenever it's relevant that the encoded version in the database is an actual JSON (i.e. json and jsonb for postgres).
  • the array cast name is confusing when working with a database like Postgres which has a native ARRAY[] type because it's effectively the same as json
  • the encoding for ARRAY[] is very specific, as detailed at https://www.postgresql.org/docs/current/arrays.html#ARRAYS-INPUT
    • seems a most appropriate way would be to have a postgres_array cast

That said, I agree with @driesvints

Since it seems that this isn't really a bug but more of a request for more native Postgres support, I think it's best that this is moved to the ideas repo.

Btw, there's is laravel/ideas#1354 and probably others I couldn't find quickly.

@gareth-ib
Copy link
Author

this array cast breaks ability to use eloquent on ARRAY[] field. So because of this hack that stores json into text field, now any project with actual ARRAY[] fields can't use a model generator at all, because eloquent doesn't have a concept of what the actual field type is, separate from the translation concept that $cast does.

So from my point of view, sorta like the postgres_array concept @mfn said, I feel that the original array idea which is actually json, should be text_json_array or something along those lines.

It kinda seems like if there is going to be faked ideas then they should be inside the database specific classes, rather than the generic classes. Let alone that if MySQL added actual array fields, which is apparently part of the SQL Standard. Then this faked array as json in a text field would be breaking MySQL in the same way it's breaking PostGres now. And apparently it's been brought up to MySQL people... no idea on the traction.

If cast is essentially a data validation concept on the PHP level, then that seems it shouldn't be also the storage format system as well.

@gareth-ib
Copy link
Author

yeah that custom castings idea you referenced is what I picture the current array functionality should be under... cuz it's not what actually exists in the database, it's a custom idea that only exists in the PHP level.

So that custom casting that was built into eloquent is breaking generic model/database functionality...
I think the way to fix it would need the casting array system to check if the field type is an actual array or not. Which code structure logically would happen inside a Postgres class that would override the generic functionality. Because this current array concept requires an assumption that the field is a text datatype field.

So is eloquent intended to hold custom logic that requires non-uniform aspects to be added to in sync between the DB and the PHP? Cuz in this scenario, the array cast can never be in a generated model, because it breaks when the field is actually an array. So only is able to exist when a person manually adds the array cast to a field. Which doesn't seem at all like what logic a generic system should be structured as.

@mfn
Copy link
Contributor

mfn commented Dec 28, 2019

Please see for a current PR proposal to support custom casting => #30958

@gareth-ib
Copy link
Author

@mfn seems taylor is blocking fixes...

@vallerydelexy
Copy link

its 2024.
laravel still doesnt support array.

@prosite01
Copy link

its 2024. laravel still doesnt support array.

Thank you for saving my time.

@mfn
Copy link
Contributor

mfn commented Feb 4, 2024

Isn't this possible with custom caster, which make sure the raw attribute format matches what pgsql needs?

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

No branches or pull requests

6 participants