tpetry / laravel-postgresql-enhanced

Support for many missing PostgreSQL specific features
MIT License
772 stars 31 forks source link

feat: support arrays via modifier #29

Closed jaulz closed 1 year ago

jaulz commented 2 years ago

This PR allows you to create the given type as an array with customisable depth.

tpetry commented 2 years ago

The array depth parameter has no effect in PostgreSQL, it doesn't do anything. If you specify a depth of 2 you can still exceed that depth. It's only there for “documentation purposes”. I don't share that opinion of the PostgreSQL and would prefer removing the ability to specify a depth when it has no real effect.

jaulz commented 2 years ago

Sure, I just removed it.

tpetry commented 2 years ago

The implementation has sadly the same problem as mine which I did last year:

Schema::create('foo', function(Blueprint $table) {
  $table->string('str')->nullable();
  $table->bigInteger('col')->array();
});

Schema::table('foo', function(Blueprint $table) {
  $table->string('str')->nullable(false)->change();
});

// Doctrine\DBAL\Exception
// Unknown database type _int8 requested, Doctrine\DBAL\Platforms\PostgreSQL100Platform may not support it.

Doctrine doesn't know any array type and we would have to create types for every PostgreSQL type as an array, otherwise the native Laravel migrations do not work anymore. In my tests (if I remember correctly) also changing the column sometimes resulted in no change being done for array columns or being converted to string columns.

Also the string('col')->array() mode will work currently but that's bad luck because Doctrine has exactly this type hardcoded. In PostgreSQL it's named '_varchar' but Doctrine will simply interpret this as a string type which means it may handle it completely wrong in some cases.

The only way I had seen last year was writing a completely new table diff algorithm replacing the doctrine one to add support for array types. And support more stuff that is currently not supported by Doctrine.

jaulz commented 2 years ago

Oh, thanks a lot for the great analysis! So it definitely seems that it's not worth it to implement it and instead rely on the jsonb type instead.

tpetry commented 2 years ago

Oh array types are totally worth it to support them, it‘s just not easy to do in Laravel. However, I‘ll keep the PR open. Maybe someone will have a great idea on how to solve the problem without replacing Doctrine‘s schema diffing algorithm.

jaulz commented 1 year ago

@tpetry I wonder if you have any nice way right now to use array types anyway. At the moment I wrote my own "raw" type to support any custom type but that requires me to enhance the schema.

tpetry commented 1 year ago

It's not documented yet, but you can also use the domain type method of the schema's blueprint with any other type ;) But when trying to change the table later with Laravel's $column->..->change() functionality it won't work anymore as doctrine's dbal doesn't understand array types. I have to write a complete new migration provider at some point - which is dozens of hours of work.

jaulz commented 1 year ago

Okay, thanks for your feedback 😊 Usually, I don't change columns anyway so that will be fine for me.

tpetry commented 1 year ago

Adding support for any array type is very very hard in PostgreSQL. And to be honest, there isn't a real benefit of text[] compared to jsonb - except the automatic type checking that can be done manually with JSON schema constraints. And jsonb has also a lot more operators to search within JSON documents compared to arrays.

However, integer[] is a very useful type because the intarray extension is adding a lot of nice stuff for this specific data type - including the awesome querying support like tags @@ '3&4&(5|6)&!7'.

So the integerArray type has been added and the idea of adding generic PostgreSQL arrays has been abandoned for now.

vishnupeas commented 3 weeks ago

I was thinking of making varchar[] work, but reading though all these. I guess I will stick with json.