yii2 icon indicating copy to clipboard operation
yii2 copied to clipboard

ActiveDataProvider::prepareModels: wrong queries when using union subqueries and pagination

Open santilin opened this issue 1 year ago • 7 comments

What steps will reproduce the problem?

Create a query with a union subquery and use it with a paginated dataprovider:

$q1 = (new ActiveQuery())->from('my_table');
$q2 = (new ActiveQuery())->from('my_table');
$q1->andWhere(['id' => 2]);
$q2->andWhere(['user_id' => 4]);
$union = $this->union($q2);
// create a dataprovider with a pagination.
$d = new ActiveDataProvider( ['query' => $union, 'pagination' => .....] );
print_r ($d->getModels());

What is the expected result?

No errors should be reported.

What do you get instead?

SQLSTATE[HY000]: General error: 1 LIMIT clause should come after UNION not before Failed to prepare SQL: SELECT "my_table".* FROM "my_table" WHERE ("my_table".id=:qp0) LIMIT 10 UNION SELECT "my_table".* FROM "my_table" WHERE ("my_table".user_id=:qp2)

The problem lies in ActiveDataProvider::prepareModels():

    if (($pagination = $this->getPagination()) !== false) {
        $pagination->totalCount = $this->getTotalCount();
        if ($pagination->totalCount === 0) {
            return [];
        }
        $query->limit($pagination->getLimit())->offset($pagination->getOffset());
    }

The $query->limit should take into account that if there is a union clause, that limit should be put in the last union query:

          if (count($query->union)>0) {
              $query->union[count($query->union)-1]['query']->limit($pagination->getLimit())->offset($pagination->getOffset());
         } else {
            $query->limit($pagination->getLimit())->offset($pagination->getOffset());
         }

There ara also lots of problems with orderBy clauses and unions, but all of them can be sorted out removing all the orderBy clauses and adding just one to the last union subquery, but it would be nice if Yii2 made it for us.

santilin avatar Aug 01 '24 12:08 santilin

Working on this issue, I find impossible to use a union query for a gridview with an activecord, because when I click on the header of a cell to sort its contents, the orderby is added to the main query and I get the error:

SQLSTATE[HY000]: General error: 1 ORDER BY clause should come after UNION not before

Using Yii2.0.52-dev and php8.1 and mysql 8.0.39

santilin avatar Aug 01 '24 13:08 santilin

The final function that fixes all these problems would be:

/**
 * {@inheritdoc}
 */
protected function prepareModels()
{
    if (!$this->query instanceof QueryInterface) {
        throw new InvalidConfigException('The "query" property must be an instance of a class that implements the QueryInterface e.g. yii\db\Query or its subclasses.');
    }
    $query = clone $this->query;

    $has_union = $query->union && count($query->union);
    if (($pagination = $this->getPagination()) !== false) {
        $pagination->totalCount = $this->getTotalCount();
        if ($pagination->totalCount === 0) {
            return [];
        }
        if ($has_union) {
            $query->union[count($query->union)-1]['query']->limit($pagination->getLimit())->offset($pagination->getOffset());
        } else {
            $query->limit($pagination->getLimit())->offset($pagination->getOffset());
        }
    }
    if (($sort = $this->getSort()) !== false) {
        $query->addOrderBy($sort->getOrders());
    }
    if ($has_union) {
        if ($query->orderBy) {
            $query->union[count($query->union)-1]['query']->addOrderBy($query->orderBy);
            $query->orderBy = null;
        }
    }

    return $query->all($this->db);
}

santilin avatar Aug 01 '24 14:08 santilin

Do you have time for a pull request?

samdark avatar Aug 01 '24 18:08 samdark

Sure, I'll do it ASAP.

santilin avatar Aug 01 '24 20:08 santilin

From which branch should I make the pr? How can I add a tests for that pr? Note: I'll do it under a different user: cepaim

cepaim avatar Aug 16 '24 10:08 cepaim

master, just add test files.

samdark avatar Aug 16 '24 18:08 samdark

Have a look at this pr: https://github.com/yiisoft/yii2/pull/20246

santilin avatar Aug 18 '24 15:08 santilin

It could work.

$q1 = (new ActiveQuery())->from('my_table');
$q2 = (new ActiveQuery())->from('my_table');
$q1->andWhere(['id' => 2]);
$q2->andWhere(['user_id' => 4]);

$unionQuery = (new Query())->from(['u' => $q1->union($q2)])->select('*');

$dataProvider = new ActiveDataProvider(
    [
        'query' => $unionQuery,
        'pagination' => [
            'pageSize' => 10,
        ],
   ],
);

terabytesoftw avatar Jan 16 '25 11:01 terabytesoftw