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] Relations doesn't use connection of parent model. #9355

Closed
deftomat opened this issue Jun 21, 2015 · 30 comments
Closed

[Eloquent] Relations doesn't use connection of parent model. #9355

deftomat opened this issue Jun 21, 2015 · 30 comments
Labels

Comments

@deftomat
Copy link

I try to get models with their relation from connection, which is chosen dynamically:

User::on('another-db')->with('article')->get();

Unfortunately, instance of related model, which is created in the Model::hasMany() function does not contain this connection.

Solution is to modify function as follows:

public function hasMany($related, $foreignKey = null, $localKey = null)
{
    $foreignKey = $foreignKey ?: $this->getForeignKey();

    $instance = new $related;

    // FIX
    if ($instance->getConnection() == null)  {
        $instance->setConnection($this->connection);
    }

    $localKey = $localKey ?: $this->getKeyName();

    return new HasMany($instance->newQuery(), $this, $instance->getTable().'.'.$foreignKey, $localKey);
 }

This solution may be used also in belongsTo(), belongsToMany(), etc.

I can create the pull request, but I think this solution may be undesirable if the user wants to define the relation between the parent model from another-db connection and the related model from default connection. This solution override a default connection in related model.

However, I believe that relations between models from the same database are more likely than relations between models from different databases.

@GrahamCampbell
Copy link
Member

What laravel version?

@deftomat
Copy link
Author

I try it with 5.1.2 and also with 4.1.*

@WazzaJB
Copy link

WazzaJB commented Jun 29, 2015

I am also experiencing this issue, I know this is not intended behaviour as Eloquent/Model.php Line 614 states:

// First we will just create a fresh instance of this model, and then we can
// set the connection on the model so that it is be used for the queries
// we execute, as well as being set on each relationship we retrieve.

I am experiencing this on version 4.2.

At the advise of Joachim Martinsen on Slack, I also tried eager loading the relationships to alleviate this with no success.

@jorgercosta
Copy link

Also looking for fix for this

@GrahamCampbell
Copy link
Member

Also just ran into this. 4.1

4.1 is not supported.

@GrahamCampbell
Copy link
Member

@deftomat That fix you provide looks wrong to me.

@GrahamCampbell
Copy link
Member

I am also experiencing this issue, I know this is not intended behaviour as Eloquent/Model.php Line 614 states:

That is for the on method, not for relations.

@GrahamCampbell
Copy link
Member

Each model defines it's own connection, and does not try to use that of another model. If you want this behaviour, you need to roll your own relation functions.

@WazzaJB
Copy link

WazzaJB commented Aug 15, 2015

Although probably not the best solution to this issue, I did manage to come up with a workaround.

Store the current database, then set the current database to be the database of the relation.

$default = Config::get('database.default');
Config::set('database.default', 'mysql_master');

After you've done your query, reset the current database in current config.

Config::set('database.default', $default);

Hope this helps anyone with a similar issue.

@frdani
Copy link

frdani commented Sep 28, 2015

For those who still has this problem:

function relation(){
   $foreignKey = 'fKey';
   $localKey  = 'lKey';

   $instance = new Model();
   $instance->setConnection($this->getConnectionName());

  return new HasOne($instance->newQuery(), $this, $instance->getTable().'.'.$foreignKey, $localKey);
}

I hope it helps!

@frdani
Copy link

frdani commented Sep 28, 2015

Of course you will have to adapt that to suit your type of relation....
This should work with eager loading too....
but I still think it would be great to have a documented final solution to this.

@ctrlaltdylan
Copy link

Would be great if:

$modelInstance->setConnection('different-database')->relation()->first();

was possible.

@davisonja
Copy link

I've just run into this with 5.1 also.

Instead of duplicating the wheel by rolling-your-own relation functions I suggest a method of specifying that related models should inherit (or the parent should propogate, given it seems the appropriate place for the change is in Model.php) the parent's connection. The likes of a 'propogateConnection' flag which is used in belongsTo (etc):

$instance = new $related;
if ($this->propogateConnection) {
    $instance->setConnection($this->getConnectionName());
}

@Frondor
Copy link
Contributor

Frondor commented Jul 18, 2016

Why is it closed without a solution? Model relationships aren't inheriting parent's connections, we aren't able to set a connection for them neither

@tannermccoleman
Copy link

If model relationships with pivot tables don't inherit a connection shared by both related models, there should at least be a way to set the pivot table or connection dynamically.

@manu144x
Copy link

I did a completely unorthodox way, I needed an extremely urgent fix, and since this hasn't been updated for 2 years I just added this line in the Model.php on all the relationship functions:

$instance = new $related;
$instance->setConnection($this->getConnectionName());

And it just works as expected absolutely everywhere, if there is no connection overload, it just uses the default one and that's it.

I think it should be the official fix, but until then I had to commit the Laravel framework folder to my repository, and lock the version in composer.

I hope an official fix will soon arise so I don't have to do these kinds of blasphemy.

@ctrlaltdylan
Copy link

@manu144x sometimes the unorthodox solutions are the best solutions!

@reno1979
Copy link

I also encountered this issue in 5.3.19 :(

I hope there will be a real fix for this

@reno1979
Copy link

@GrahamCampbell @taylorotwell @themsaid @BartHuis

Could this issue please be reopend?
I've encounter multiple issues this week, all caused by this underlying mecanism/issue.

I 'solved' it with changing the default database connection and restoring it, but this is going to introduce unwanted effects eventually.

Adding this 'fix' to relations in a own made model should work:

$instance = new $related;
$instance->setConnection($this->getConnectionName());

When a Laravel extension (which creates relations in your Models etc) is used, this extension should also apply this 'fix'. :(

But a real solution would be much nicer. I saw a proposal for a propegateConnection by @davisonja
Something like that could work. Or similar:

App\myModel::on('mysql2', true); /* True for propegate connection */

@themsaid
Copy link
Member

@reno1979 it's not a bug, it's by design.

@BartHuis
Copy link

@themsaid and how do you have to work around in our case?

@BartHuis
Copy link

@themsaid becouse when you set the connection, and the script fails, your connection stays on the wrong one.

@themsaid
Copy link
Member

Currently every model sets its own connection and there's no way to force the parent connection, I'm checking with Taylor if adding such functionality would be a good idea.

@BartHuis
Copy link

would be nice, would be a solution for this one also then: dimsav/laravel-translatable#283

@avasa
Copy link

avasa commented Feb 27, 2017

I had the same issue when my belongsToMany relationship wasn't working off the parent connection.

My solution was to override the belongsToMany method on the model trying to establish .

See answer below.

http://stackoverflow.com/a/42497184/932696

@BartHuis
Copy link

@avasa I didn't read everything yet, but do you use the most recent laravel version? Much things are already fixed...

@Frondor
Copy link
Contributor

Frondor commented Feb 28, 2017

This is already implemented on L5.4, although I think it should be optional by setting something like protected $inheritConnection = true; because its annoying to handle some times.

Or better yet, I like reno1979's idea of App\myModel::on('mysql2', true); /* True for propegate connection */

@avasa
Copy link

avasa commented Feb 28, 2017

@BartHuis @Frondor I am currently using version 5.2. I will try and update my package. Appriciated the feedback guys.

@fredec
Copy link

fredec commented Jun 16, 2017

@JRogaishio
Copy link

Sorry to revive a crazy old issue but this still happens in laravel 5.8-7.x.

I've found a possible solution which involves overwriting Illuminate/Database/Eloquent/Concerns/HasRelationships.php newRelatedInstance() method. This can be done in your base model and seems fairly easy.
laravel/ideas#2326

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

No branches or pull requests