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

Self join prevents DB query cache #13353

Closed
JeroenVanOort opened this issue Apr 28, 2016 · 6 comments
Closed

Self join prevents DB query cache #13353

JeroenVanOort opened this issue Apr 28, 2016 · 6 comments

Comments

@JeroenVanOort
Copy link
Contributor

It occurred to me, that any query using a self join is never to be cached by a database because, although the query parameters remain equal, the query isn't the same every time it's executed. This is because Eloquent generates a table alias based on the microtime (BelongsToMany, BelongsTo, HasOneOrMany).

Do you think it would be possible to generate the table alias based on something that doesn't change a much? Like an md5 of the table name, for example. This would allow for the query to be cached by the database itself.

I'd be happy to make a pull request, but I can't oversee all implications of what I'm suggesting; it might have some side effects or break BC, I don't know. Therefore, I thought it'd be a good thing to submit an issue first.

@prohalexey
Copy link
Contributor

I think i can be a static variable with increment value

@JeroenVanOort
Copy link
Contributor Author

And why would you do that instead of and md5 of the table name? It's seems much more complex to me and I don't understand the advantage of it.

@prohalexey
Copy link
Contributor

prohalexey commented Apr 30, 2016

Because you can do many self join queries, so eloquent must have ability to identify columns in mysql result

@JeroenVanOort
Copy link
Contributor Author

JeroenVanOort commented May 2, 2016

Ok, this is just the kind of thing I was looking for; I didn't realize that an md5 of the table name would go wrong when working with multiple self joins. Thank you for letting me see that.

As for a solution: I think that in every relation class named above, we need a static property containing an associative array containing the table names as keys and a count of the times getRelationCountHash is called for that relation and table as values. The return value of getRelationCountHash would be based upon a md5 of the table name and the count. What do you think about that?

I could make a pull request for these changes, but I'm not that good with writing tests for them. Could anyone help me with that?

@prohalexey
Copy link
Contributor

This behavior can be done, by replace original function with function above, and i think you do not need array, just incremental variable.

static protected $self_count_join = 0;

public function getRelationCountHash()
{
    return 'self_'.static::$self_count_join++;
}

@JeroenVanOort
Copy link
Contributor Author

Maybe the cache could be more effective when using the array, but this simple solution will work well in most cases. I've just tried this on a staging server and is does indeed drastically decrease the load on the DB. Response times have quartered.

Since this approach is much more simple than mine, I will make a pull request without tests and hope for the best. No current tests will fail.

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

No branches or pull requests

2 participants