yiisoft / active-record

Active Record database abstraction layer
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
70 stars 29 forks source link

Can not call `ActiveRecord::all()` method with specified connection for database with different scheme #89

Closed HaruAtari closed 3 months ago

HaruAtari commented 4 years ago

I have two database connections. The main connection (\Yii::$app->db) is empty now. And the second which connected to a database with existed tables. Also I have an activeRecord model for an existed table from the second connection.

My code:

$connection2 = new Connection(/* ... */);
$connection2->open;
$records = MyActiveRecordModel::find()->all($connection2);

When I run it, I have an error: The table does not exist: {{%my_active_record_model}}. But how I said this table exists.

The method \yii\db\Query::all() takes data (rows) from the second database successfully. After that it try to populate records into activeRecord models. It calls \yii\db\ActiveQuery::populate() -> \yii\db\ActiveQueryTrait::createModels() -> \yii\db\ActiveRecord::populateRecord(). And here we have a problem.

This method takes a list of columns from the table schema from the \Yii::$app->db connections, but not from the connection, which was specified in all() call.

So I can't specify connection to take data from database with scheme different from the current connection.

Additional info

Q A
Yii version 2.0.32
PHP version any
Operating system any
fcaldarelli commented 4 years ago

You can override MyActiveRecordModel getDb method. If you secondary db connection is named db2, this is an example:

class MyActiveRecordModel extends ActiveRecord
{
    public static function getDb()
    {
        return Yii::$app->getDb2();
    }
HaruAtari commented 4 years ago

@FabrizioCaldarelli Thank you for your answer. But if I do that, all queries will be use the second connection. But I need to use it only for a single query

I solve my case by changing of \Yii::$app->db. But it seems like a bug (in architect or in the docs).

samdark commented 4 years ago

@HaruAtari any way it can be fixed without breaking things?

samdark commented 4 years ago

Without taking backwards compatibility in account, I'd say it's a wrong behavior that given explicit DB connection it uses schema from another connection.

HaruAtari commented 4 years ago

@samdark I don't know. I'm not enough good in this part of the framework.

bizley commented 4 years ago

I took a deeper look at this and I think with the current implementation of AR it would be very difficult to fix it (just too many static methods all over the place), not to mention the most probable BC break will be introduced. I don't think fixing this unusual case is worth it here (hopefully Yii 3 can do something about it). Adding it to the "known problems" list would be handy. I can only suggest to @HaruAtari some custom by-pass of the problem like having two AR models for this or maybe configuring the instance on-the-fly with constructor or config array.

samdark commented 4 years ago

Moved issue to standalone AR implementation. @HaruAtari if you know a good backwards compatible way to change it in Yii 2, let us know.

brussens commented 4 years ago

I certainly understand that the thoughts that came to my mind are an evil crutch, but nevertheless, this option is most likely to work (I have not checked). The idea is to throw the connection object through the entire method chain, up to \yii\db\ActiveRecord::populateRecord() to process the connection there as follows:

public static function populateRecord($record, $row, $db = null)
 {
        $columns = static::getTableSchema($db)->columns;
        foreach ($row as $name => $value) {
            if (isset($columns[$name])) {
                $row[$name] = $columns[$name]->phpTypecast($value);
            }
        }
        parent::populateRecord($record, $row);
}

And already at \yii\db\ActiveRecord:

public static function getTableSchema($db = null)
 {
        $db = $db ?? static::getDb();
        $tableSchema = $db
            ->getSchema()
            ->getTableSchema(static::tableName());

        if ($tableSchema === null) {
            throw new InvalidConfigException('The table does not exist: ' . static::tableName());
        }

        return $tableSchema;
 }

This way we can maintain backward compatibility, however, at the cost of the fact that in \yii\db\Query::populate() we will have a completely unnecessary parameter.

samdark commented 4 years ago

That won't be backwards compatible so can't be done in Yii 2. This way you will force updating all the AR classes that implement populateRecord or else you'll get an error:

Warning: Declaration of MyModel::populateRecord($record, $row) should be compatible with ActiveRecord::populateRecord($record, $row, $db = null)
brussens commented 4 years ago

Then one of the dirty options is to do something like that.

class ActiveQuery
{
    protected $db = null;

    public function all($db = null)
    {
        $this->db = $db;
        return parent::all($db);
    }

    public function populateRecord($record, $row)
    {
        $modelClass = get_class($model);
        if ($this->db === null) {
             $modelClass::populateRecord($record, $row);
        } else {
            $columns = $this->db->getSchema()
            ->getTableSchema($modelClass::tableName())->columns;
            foreach ($row as $name => $value) {
                if (isset($columns[$name])) {
                    $row[$name] = $columns[$name]->phpTypecast($value);
                }
            }
            BaseActiveRecord::populateRecord($record, $row);
        }
    }
}

ActiveQueryTrait

trait ActiveQueryTrait
{
    protected function createModels($rows)
    {
        if ($this->asArray) {
            return $rows;
        } else {
            $models = [];
            /* @var $class ActiveRecord */
            $class = $this->modelClass;
            foreach ($rows as $row) {
                $model = $class::instantiate($row);
                $this->populateRecord($model, $row);
                $models[] = $model;
            }
            return $models;
        }
    }
}

It's shameful, but it should solve the problem.

Commifreak commented 1 year ago

I still came across that issue:

Yii 2.0.47:

Projekt::find()
                                ->asArray()
                                ->one( $myDb);

The Yii$app->db does NOT have the whole table. But $myDb has.

But:

2023-05-25 14:15:02 [-][-][-][error][yii\base\InvalidConfigException]
yii\base\InvalidConfigException: The table does not exist: projekt in
/var/www/vendor/yiisoft/yii2/db/ActiveRecord.php:442
Stack trace:
#0
/var/www/vendor/yiisoft/yii2/db/ActiveRecord.php(513):
yii\db\ActiveRecord::getTableSchema()
#1
/var/www/vendor/yiisoft/yii2/db/ActiveQueryTrait.php(123):
yii\db\ActiveRecord::populateRecord()
#2
/var/www/vendor/yiisoft/yii2/db/ActiveQuery.php(219):
yii\db\ActiveQuery->createModels()
#3
/var/www/vendor/yiisoft/yii2/db/ActiveQuery.php(306):
yii\db\ActiveQuery->populate()
#4
/var/www/common/components/notification/NotificationInterface.php(314):
yii\db\ActiveQuery->one()

Whats the state of the "bug"?

xepozz commented 1 year ago

@Commifreak try ask in yii2 repo. this one for yii3

Commifreak commented 1 year ago

Ok, will do it. Just wondering, as the author was using Yii 2 as well.

samdark commented 1 year ago

Yes. The issue was initially opened in Yii2 then moved here.

Tigrov commented 5 months ago

Currently you can specify a db connection when create an ActiveQuery instance.

$userQuery = new ActiveQuery(User::class, $db);
$users = $userQuery->all();
Tigrov commented 3 months ago

Now you can define method withConnection() inside the model and use it to change connection.

$userQuery = new ActiveQuery((new User())->withConnection($db2));
$users = $userQuery->all();