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

Static relations #17

Open
SamMousa opened this issue Oct 28, 2018 · 7 comments
Open

Static relations #17

SamMousa opened this issue Oct 28, 2018 · 7 comments

Comments

@SamMousa
Copy link
Contributor

Currently relationship definitions in AR are defined as non static getters. In reality an AR class represents a table and an instance of that class represents a record.

Relations are defined at the table level, but because AR uses non-static getters our query classes need to instantiate a dummy model for greedy queries.

Requirements for AR relationships:

  • One definition for lazy and greedy loading
  • Support (cached) retrieval via magic getter
  • Support getting a query object (currently via the getter that defines it)

Pros of current approach:

  • Uses "native" magic getters (with a small extension)
  • Has code completion for the getters since they are normal functions

Cons:

  • No way to get the relationships without instantiating a dummy model
  • No way to enumerate relations

In Yii1 we had a single function defining the relationships but we also had the dummy model due to lack of late static binding. This allowed for enumeration but had other down sides.

What if, in Yii3 we try to get the best of both? Suppose we define relationships as static functions:

public static function relatedCustomers()
{
    return Has::many (Customer::class, ['id' => 'customer_id']);
}

Internally we would have a sound definition of the relationship, from this we can:

  • build a lazy query
  • build a greedy query
  • enumerate via reflection

From a consumer point of view nothing changes:

  • getCustomers via __invoke()
  • ->customers via __get()

Pros:

  • Relations are now static
  • No need for dummy instances
  • OO definition of relationship unrelated to the query object

Cons:

  • no auto complete for the getter, so could require additional annotations.
  • breaks BC
@SamMousa
Copy link
Contributor Author

This issue is to collect different point of views so please provide yours!

@samdark
Copy link
Member

samdark commented Oct 28, 2018

  1. Breaking BC is OK if there are pros and no other cons.
  2. I don't like that getCustomers() and ->customers are currently different. That isn't explicit.
  3. Ability to enumerate relations is a good addition. It was asked multiple times to have it.

@SamMousa
Copy link
Contributor Author

SamMousa commented Oct 28, 2018

_ 2. Would become more explicit when we get resolvable properties in core.

Some other thing we lose with my proposal is things like forcing joins in a relation, not sure if that is something people do?
Is there anything in the current query object that you can't or shouldn't be doing in a relation?

@samdark
Copy link
Member

samdark commented Oct 28, 2018

Some other thing we lose with my proposal is things like forcing joins in a relation, not sure if that is something people do?

Do you mean joinWith()? Why?

Is there anything in the current query object that you can't or shouldn't be doing in a relation?

Nothing I can think of immediately.

@SamMousa
Copy link
Contributor Author

I'll try to work on a simple PoC

@SamMousa
Copy link
Contributor Author

SamMousa commented Nov 6, 2018

@samdark could you move this to active-record? All these repos still taking a bit of getting used to :-/

@samdark samdark transferred this issue from yiisoft/db Nov 10, 2018
@Insolita
Copy link

Insolita commented May 10, 2019

+1 for this idea
Current realization of relations has a big disadvantage - it make not possible to detect existed model relations without it loading, and also internal relation inspecting is very complex. In yii1 we have explicit relation definitions belongs_to, has_one, has_many etc. At now even we get relation by name, we can't explicitly understand what kind these relation - hasOne relation definition is same as has_one and belongs_to. We must also check primary keys. If query has "via" definition, we can't explicitly understand is these classical junction, or "through" relation. Firstly it make harder to build some abstract admin solutions in django-admin style or gii genearation with better relation-based ui desicions
And generally work with relations looks difficult https://github.com/la-haute-societe/yii2-save-relations-behavior/blob/master/src/SaveRelationsBehavior.php

In issues i see feature requests for some features from eloquent like sync junction relations or polymorphic relations support, and as the fact - in eloquent relation definitions store more information https://github.com/illuminate/database/tree/master/Eloquent/Relations

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

3 participants