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

Activedataprovider set limit inside of query with union but should outside #65

Open
tunforjob opened this issue Nov 10, 2016 · 15 comments

Comments

@tunforjob
Copy link

What steps will reproduce the problem?

$query = Product::find()->where(['id' => 1]);
$query2 = Product::find()->where(['id' => 2]);
$query-> union($query2);
$provider = new ActiveDataProvider([
'query' => $query,
    'pagination' => [
        'pageSize' => 12,
    ],
]);
$provider->getModels();

What is the expected result?

(SELECT * FROM product WHERE id=1) UNION ( SELECT * FROM product WHERE id=2 ) LIMIT 12

What do you get instead?

(SELECT * FROM product WHERE id=1 LIMIT 12) UNION ( SELECT * FROM product WHERE id=2 )

Additional info

Q A
Yii version 2.0.?
PHP version
Operating system
@SilverFire
Copy link
Member

SilverFire commented Nov 11, 2016

It's a specific use case.
We can solve it somehow like this (inside of ActiveDataProvider):

// concept, did not test it
function wrapQuery(query)
{
    $newQuery = new get_class($query);
    $query->from(['originalQuery' => $query])

    return $newQuery; // and then use this $newQuery for pagination
}

I'm not sure whether we should add this complexity to ActiveDataProvider.
@tunforjob you can try to use my concept in your application code before passing $query to ActiveDataProvider

@klimov-paul
Copy link
Member

Relates #7992

@dynasource
Copy link
Member

It's a specific use case.

Not if you look at the topic @klimov-paul supplied. We should develop a solution that matches the expected result of the issue introduction.

@SamMousa
Copy link
Contributor

Would it make sense to:

  1. Remove ->union() from Query.
  2. Introduce: UnionQuery::__construct($q1, $q2)?

That way create a more explicit and intuitive syntax.

@samdark
Copy link
Member

samdark commented Mar 30, 2017

Yes. That's a good idea.

@dynasource
Copy link
Member

agree, thats intuitive

@TurningTide
Copy link

any progress on this?

@SilverFire
Copy link
Member

This issue is scheduled on future milestone. In you would like to help use with this issue - submit a pull request, we will highly appreciate it.

@pdwjun
Copy link

pdwjun commented Jun 9, 2017

SilverFire's solution work fine.

 protected function wrapQuery($query)
    {
        $newQuery = new ActiveQuery($query->modelClass);
        $newQuery->from(['originalQuery' => $query]);

        return $newQuery; // and then use this $newQuery for pagination
    }

@githubjeka
Copy link
Contributor

I vote for fixes by SamMousa
to introduce UnionQuery::__construct($q1, $q2) or somelike this
But need think about API for this class:

  • alias for union table
  • and what this class should return ActiveQuery, Query...
  • how uses filters after union
 $query->andFilterWhere(//... );

For now, my code look this for this case

https://github.com/githubjeka/tracker-issues/blob/6a53251e1892f50212608162b0258ebba5b08e1f/models/DocumentSearch.php#L51

@SamMousa
Copy link
Contributor

@githubjeka those issues sound like they are solvable.

ActiveQueryInterface mainly adds with, via and findFor, all of these are not needed for the union.

To make sure a union query can still return ActiveRecord objects we could even do something really cool (just thought of it, might have some drawbacks):

  1. A union query consists of n query objects.
  2. Assign a unique identifier to each query object.
  3. Add a constant to the select clause of each query.
  4. Execute the query.
  5. For each result row, remove the identifier from (2) and then call populate($rows) on the correct query object.

So suppose I have 2 ActiveQuery objects one for model Car and one for model Bike, each of these have the select clause set to ['id', 'name']. The invidual queries look like this:

SELECT id, name FROM car;
SELECT Id,name FROM bike;

The union query would do this:

SELECT "0" as queryObj, id, name FROM car
UNION
SELECT "1" as queryObj, id, name FROM bike;

Then after retrieving the results it would pass each row to populate and get the resulting object.
One downside to this approach is that it will create duplicates in the query results which are normally filtered by ActiveQuery.

Of course this approach could be simplified, in case both ActiveQuery instances are of the same class and have the same modelClass.

What we need to do is figure out some usages and expected results, for example:

// Expected result get all cars as AR models.
(new UnionQuery(Car::find(), Car::find()))->all();

// Expected result get cars with id < 10 or > 20 as AR models.
(new UnionQuery(Car::find()->where(['<', 'id', 10]), Car::find()->where(['>', 'id', 20])))->all();

// Expected result get all cars as arrays, followed by all cars as AR models. ?
(new UnionQuery(Car::find()->asArray(), Car::find()))->all();

// Expected result get all cars with id < 10 and all cars with (id > 20 and with the Owner relation populated)
(new UnionQuery(Car::find()->where(['<', 'id', 10]), Car::find()->with('Owner')->where(['>', 'id', 20])))->all();

Some of these uses, for example the last one, will be hard to properly implement in ALL cases; however this does not seem like a proper use to me that needs in depth support.
In most cases where you use a UNION you have to very similar queries with results of the same type and in the same format.

@m2fik
Copy link

m2fik commented Mar 20, 2018

@SamMousa
When I change the place of union in code of QueryBuilder.php
from
`$sql = $this->buildOrderByAndLimit($sql, $query->orderBy, $query->limit, $query->offset);

    if (!empty($query->orderBy)) {
        foreach ($query->orderBy as $expression) {
            if ($expression instanceof Expression) {
                $params = array_merge($params, $expression->params);
            }
        }
    }
    if (!empty($query->groupBy)) {
        foreach ($query->groupBy as $expression) {
            if ($expression instanceof Expression) {
                $params = array_merge($params, $expression->params);
            }
        }
    }

    $union = $this->buildUnion($query->union, $params);
    if ($union !== '') {
        $sql = "($sql){$this->separator}$union";
    }`

to

`$union = $this->buildUnion($query->union, $params);

    if ($union !== '') {
        $sql = "($sql){$this->separator}$union";
    }
    
    $sql = $this->buildOrderByAndLimit($sql, $query->orderBy, $query->limit, $query->offset);
    if (!empty($query->orderBy)) {
        foreach ($query->orderBy as $expression) {
            if ($expression instanceof Expression) {
                $params = array_merge($params, $expression->params);
            }
        }
    }
    if (!empty($query->groupBy)) {
        foreach ($query->groupBy as $expression) {
            if ($expression instanceof Expression) {
                $params = array_merge($params, $expression->params);
            }
        }
    }`

it works the union with sorting, limit, paging and it products a ActiveRecord and not array.
Hope it helps what I found.

now the result of this code
$query = Product::find()->where(['id' => 1]); $query2 = Product::find()->where(['id' => 2]); $query-> union($query2); $provider = new ActiveDataProvider([ 'query' => $query, 'pagination' => [ 'pageSize' => 12, ], ]); $provider->getModels();
is
(SELECT * FROM product WHERE id=1) UNION ( SELECT * FROM product WHERE id=2 ) LIMIT 12

@sobwoofer
Copy link

sobwoofer commented May 15, 2018

@m2fik thanks, it helped me!!, my code became working.
And how to do it accurately and correctly that not edited vendor QueryBuilder?

@skvarovski
Copy link

problev actualy!!!
thanks for patch @m2fik

@samdark samdark transferred this issue from yiisoft/yii2 Apr 24, 2019
@fadilsyeha
Copy link

fadilsyeha commented Sep 29, 2021

you can try this way, it's work for me.

public function yourFunction()
{
    $query = Product::find()->where(['id' => 1]);
    $query2 = Product::find()->where(['id' => 2]);
    
    $unionQuery = self::find()->from(['dummy_name' => $query->union($query2)]);
    
    $provider = new ActiveDataProvider([
    'query' => $unionQuery,
      'pagination' => [
          'pageSize' => 12,
      ],
    ]);

    return $provider;
}

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