gino icon indicating copy to clipboard operation
gino copied to clipboard

Subqueryloader

Open jekel opened this issue 7 years ago • 12 comments

SubqueryLoader is used to load Models from sqlalchemy aliased queries, opposite of #326 which works only on aliased gino Models Common usecase for this loader will be - when you will have complex query with subquery, aggregated columns, etc... and you need to get result as Model object defined inside subquery

Previus discussion can be found here #323

jekel avatar Sep 06 '18 10:09 jekel

Pull Request Test Coverage Report for Build 1214

  • 19 of 19 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.008%) to 98.275%

Totals Coverage Status
Change from base Build 1209: 0.008%
Covered Lines: 3987
Relevant Lines: 4057

💛 - Coveralls

coveralls avatar Sep 06 '18 10:09 coveralls

As discussed in #323, I'm not convinced that this is the complete solution. The use case in the unit test can be achieved in the latest version of Gino already. We do not need model and corresponding_column to get the column in subquery, as columns property of the subquery already has everything, as in https://github.com/fantix/gino/pull/326/files#diff-f6247715790dd30f031c9c3d67d5814eR148. We do need better support for SQLAlchemy subqueries, or a wrapper. I'll come up with a good use case first.

wwwjfy avatar Sep 09 '18 15:09 wwwjfy

We do need better support for SQLAlchemy subqueries, or a wrapper

Why not? New users may need this functionality when porting their application on asyncio.

I'll come up with a good use case first.

Can you show me please also how can i achive the same without using corresponding_column in new use cases?

jekel avatar Sep 09 '18 16:09 jekel

We "do" need, not we "don't" need. 😂

alias.corresponding_column(User.id) can be replaced by alias.columns.id, so we don't need User to be passed to the loader. I'm not saying it can cover all cases, but at least part of them.

wwwjfy avatar Sep 09 '18 16:09 wwwjfy

@wwwjfy Sorry, i have read it wrong :)

alias.corresponding_column(User.id) can be replaced by alias.columns.id

what happens when there will be several models inside subquery? please take it mind when creating new use case

jekel avatar Sep 09 '18 16:09 jekel

Exactly. That's the cases I'd like to explore, so as to give users straightforward loader and least surprise. Thanks 😊

wwwjfy avatar Sep 09 '18 16:09 wwwjfy

Hi @wwwjfy , how is going your research?

jekel avatar Oct 17 '18 10:10 jekel

Sorry for the silence!

I just picked up a few issues again and will work out something this weekend.

wwwjfy avatar Oct 17 '18 16:10 wwwjfy

I created #365, trying to show what can be done in the description of this issue, for subqueries and aggregate functions. Basically, it won't be hard as long as we keep the reference of columns and/or models in a subquery and pass them to loaders.

wwwjfy avatar Oct 21 '18 15:10 wwwjfy

I don't know if this can meet your needs. If not, an example will be very helpful.

wwwjfy avatar Oct 21 '18 15:10 wwwjfy

@wwwjfy i have found the case your code does not cover - complete model loading from subquery without specifying all columns explicitly

jekel avatar Oct 23 '18 20:10 jekel

@jekel I could achieve this:

async def test(user):
    ua = User.alias()
    query = select([ua]).alias()
    outer_query = select([text('1'), query])
    result = await outer_query.gino.load(ua).all()
    assert len(result) == 1
    assert result[0].id == user.id

Is it what you said?

wwwjfy avatar Oct 24 '18 02:10 wwwjfy