Framework: Eloquent models isGuardableColumn() calls fail on models that have a schema name in the table property

Created on 11 Aug 2020  路  7Comments  路  Source: laravel/framework


  • Laravel Version: 6.18.35
  • PHP Version: 7.2.24
  • Database Driver & Version: mysql 5.7

Description:

I've been using Laravel in a complex database setup pretty successfully for a while now by specifying the database name in my models' $table properties.

    protected $table = 'schema_name.table_name';

As of #33777, all of my models that have defined a $guarded property instead of $fillable have stopped working with mass assignment if the model also specifies a schema name in it's $table property.

I've been successfully using eloquent in this setup for a couple years now, as it is much easier than defining all of the connections individually (I'm using Laravel to interface with up to 25 schemas on a single server). We also have cross schema relationships (legacy data is fun), and this all works wonderfully prior to #33777 having been merged.

I realize this is not a documented feature, and may not even be officially supported, but it does make my life a lot easier and this genuinely feels like a bug to me.

Steps To Reproduce:

  • Create a model that specifies a schema name in the $table property
  • Attempt mass assignment on the model. If you use a non-nullable database field you'll error out right away with an exception, something like xxx column doesn't have a default value.

Specifically I did the following

  • $ composer create-project --prefer-dist laravel/laravel=6.18.8 issue-test
  • configure a mysql database connection (database set to issuetest)
  • add two schemas to the database server (issuetest, and issuetest2)
  • $ php artisan make:migration create_test_tables
    public function up()
    {
        Schema::create('test1', function (Blueprint $table) {
            $table->bigIncrements('id');
            $table->string('unguarded');
            $table->timestamps();
        });

        Schema::create('issuetest2.test2', function (Blueprint $table) {
            $table->bigIncrements('id');
            $table->string('unguarded');
            $table->timestamps();
        });
    }
  • DB should now look something like:
    image
  • $ php artisan make:model Test1 && php artisan make:model Test2
class Test1 extends Model
{
    protected $guarded = ['id', 'created_at', 'updated_at'];
    protected $table = 'test1';
}

```php
class Test2 extends Model
{
protected $guarded = ['id', 'created_at', 'updated_at'];
protected $table = 'issuetest2.test2';
}

- Then it can be tested with tinker easily enough
![image](https://user-images.githubusercontent.com/6740074/89905992-6eed6480-dbb0-11ea-8297-2c9322965f7e.png)

### Potential Fix?
The issue appears to be all the way down in `Illuminate\Database\Schema\MySqlBuilder`. The `getColumnListing()` method does not work for tables denoted by `schema.table`.

The following change works for both models, but I'm sure there's a better way to write it, this was just a quick test. I'm also sure there's angles I'm not thinking of here.

```php
    /**
     * Get the column listing for a given table.
     *
     * @param  string  $table
     * @return array
     */
    public function getColumnListing($table)
    {
        $table = $this->connection->getTablePrefix().$table;
        $dot = strpos($table, '.');
        if ($dot !== false) {
            $database = substr($table, 0, $dot);
            $table = substr($table, $dot + 1);
        } else {
            $database = $this->connection->getDatabaseName();
        }

        $results = $this->connection->select(
            $this->grammar->compileColumnListing(), [$database, $table]
        );

        return $this->connection->getPostProcessor()->processColumnListing($results);
    }

Thanks for taking a look.

needs more info

All 7 comments

Your model has a $connection property specifically to set this. Does that not work for you?

The only issue with that seems to be that I would need to add 20+ new config items to my config/database.php file, and then update all of my models accordingly (500+). I was concerned about relationships not working, but after a quick test it seems like they work just fine. I only tested a simple one to many relationship though.

My thinking for this being a bug is that the $table property has worked with schema_name.table_name for a while now. There are many references to this being an option outside of the official documentation on laracasts forums/stackoverflow questions. It is not specified in the official documentation far as I can see, though, so I wouldn't be surprised to hear that this just isn't a supported thing.

We've decided that we won't pursue backwards compatibility for this undocumented and unintended feature sorry. You should be able to get the behavior you want by overwriting the isGuarded method perhaps.

That's disappointing, but thanks for taking a look.

We ran into this issue as well. After using laravel for many years I hadn't realized that specifying a different schema/database name on the table property wasn't documented. We ended up setting our uses of guarded to an empty array or removing them instead of switching to separate connections per schema. I did take note that we likely will need to switch to multiple connections at some point since specifying the schema with the table name isn't officially supported.

@zhuston you can still use the connection property for that

@driesvints Thank you.

Primarily I was looking for the path of least resistance in order to get through with the upgrade. Our app has roughly 100 models that span 2 schemas/databases (same host). I had some concern that relations across separate connections might have issues. (they work fine now with database.tableName on the table property). I thought I read some people had issues with whereHas across connections since it creates a subquery?

I also had a minor concern for doubling up our connections to our database server with each request opening 2 connections. That would certainly be solvable if it was an issue, but for now it seemed the the easiest path was changing our usage of guarded.

Thank you for all of your work on Laravel!

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kerbylav picture kerbylav  路  3Comments

felixsanz picture felixsanz  路  3Comments

Fuzzyma picture Fuzzyma  路  3Comments

SachinAgarwal1337 picture SachinAgarwal1337  路  3Comments

shopblocks picture shopblocks  路  3Comments