yiisoft / yii-dataview

Data widgets
https://www.yiiframework.com/
BSD 3-Clause "New" or "Revised" License
38 stars 21 forks source link

Query limit not working with pager enabled. #7

Closed skworden closed 1 day ago

skworden commented 9 years ago

The pagerination pageSize overrides the limit set in the query

$query = Objects::find()->orderBy('name ASC')->limit(50);
        $dataProvider = new ActiveDataProvider([
                'query' => $query,
                'pagination' => array('pageSize' =>24)
        ]);

The above produces

find.count({"ns":"geekobjects.objects","limit":-1,"batchSize":0,"skip":-1,"flags":0,"query":{},"fields":[],"started_iterating":false})

Which in turns overrides the limit of 50 and returns all of the collections instead of 50

$query = Objects::find()->orderBy('name ASC')->limit(50);
        $dataProvider = new ActiveDataProvider([
                'query' => $query,
                'pagination' => false
        ]);

the above produces

find({"ns":"geekobjects.objects","limit":50,"batchSize":0,"skip":0,"flags":0,"query":{"$query":{},"$orderby":{"name":1}},"fields":[],"started_iterating":false})

which limits the query to 50 items but now there isn't a pager since it was set to false.

I don't know if it was intended to do this / or if this is a bug / or if I'm not doing this correctly.

skworden commented 9 years ago
use yii\data\ArrayDataProvider;

$query = Objects::find()->orderBy('name ASC')->limit(30);
        $dataProvider = new ArrayDataProvider([
            'allModels' => $query->all(),
            'pagination' => ['pageSize' => '10']
        ]);

This seems to work.
I can't get any of the mongo ways in the docs to work though.

The way i put in the initial post seems to duplicate queries and this way does not.

Sammaye commented 9 years ago

MongoDB does not support multiple limits, it simply does not exist within MongoDBs own querying capacity.

One limit will override the other. You have to do what you are there with the ArrayDataProvider.

skworden commented 9 years ago

Update:

I ended up going with this and just forgot about the limit option for now due to memory usage of the arraydataprovider.

$query = Objects::find()->orderBy('name ASC');
        $dataProvider = new ActiveDataProvider([
            'pagination' => ['pageSize' =>24],
            'query' => $query,
        ]);

The above used 5.4m of memory (yii2 is 3.4 empty) with 300k records / documents.

The only problem with this is that it uses 2 queries. one for the pager and one for the retrieving of the records.

$query = Objects::find()->orderBy('name ASC')->limit(24);
$provider = new ArrayDataProvider([
    'allModels' => $query->all(),
    'pagination' => [
        'pageSize' => 10,
    ],
]);

the above on the same documents used 534m of memory which doesn't work for anyone. The plus side is it only uses one query for the pager and retrieving of the records.

Is there a way to have the low memory usage (first block of code) and only use one query like the second code block for both pager and retrieving of documents?

klimov-paul commented 9 years ago

This issue does not relates to the MongoDB. It appears for regular DB just as well. ActiveDataProvider resets limit, while calculating total items count: https://github.com/yiisoft/yii2/blob/master/framework/data/ActiveDataProvider.php#L165 Thus limit setup has not effect. We can preserve the original query limit during total count calculation, but if you for example specify an offset - data provider still will show invalid results.

dynasource commented 8 years ago

ran into this one yesterday. It was unexpected behavior.

cebe commented 8 years ago

what is the use case for this? Usually a pagination is applied to control the limit and offset. What is the meaning of the manually specified limit?

samdark commented 8 years ago

Page size?

cebe commented 8 years ago

page size is controlled by the pagination, why set it manually?

dynasource commented 8 years ago

apart from the usecase, it was confusing to work with from a development perspective

cebe commented 8 years ago

To me it is unclear what this issue is about.

dynasource commented 8 years ago

well, its quite simple. If you are setting a limit, you are expecting it to be respected. Whether its a pagination widget or another widget.

cebe commented 8 years ago

what exactly do you expect? setting a limit and adding a pagination is ambigous. When you are on page 2 with limit 35 and a page size of 10. How many records do you want to see?

dynasource commented 8 years ago

I would expect it to behave like a ceiling, comparable with totalCount.

cebe commented 8 years ago

So you want to paginate only part of the data? e.g. the query would return 100 records and you set limit to 50. with page size 10 you get 5 pages? What is the use case for that?

rob006 commented 8 years ago

@cebe Queries with big offset can be really slow on big sets of data. Setting limit in this way can save lots of server resources.

serjgamover commented 7 years ago

@cebe Another example - deep paging in distributed system, like Elastic Search

Reference https://www.elastic.co/guide/en/elasticsearch/guide/current/pagination.html

Once the limit is set to query, the desired behavior is to paginate only limited part of data

tyler-ham commented 6 years ago

I just ran into this problem and wanted to share my use case. We offer a 100 top (most-recent) posts dataset like:

$query = Posts::find()->orderBy('created_at DESC')->limit(100);

What is the use case for that?

The ActiveDataProvider use case is to allow users to page through this top 100 list with the expectation that we'd have 10 pages of 10 posts each. With the current behavior, this top 100 list would have over 80 pages with 8000+ records.

One could argue that this should be fine (and maybe even better) since end users have access to more data and can control what page they are on, but this is an editorial/design argument and not a technical argument. And in our case, the editorial/design decision was in favor of limiting the accessible portion of the dataset to the last 100 posts.

Workaround

I am currently working around the problem by wrapping my LIMITed query with another query. I imagine that there might be some performance hit for doing this, but I haven't evaluated that so performance may or may not be an issue. Either way, it still feels like a hacky workaround:

// original limited query
$limitedQuery = Posts::find()->orderBy('created_at DESC')->limit(100);

// wrap limited query as a subquery and select everything from it
$query = Yii::createObject(ActiveQuery::class, [Posts::class])
    ->select('*')
    ->from(['limited' => $limitedQuery]);

$dataProvider = new ActiveDataProvider([
    'query' => $query,
]);

Proposed Change

I'd like to propose adding a limit property on ActiveDataProvider with the following options:

Example 1

$query = Posts::find()->orderBy('created_at DESC')->limit(100);
$dataProvider = new ActiveDataProvider([
    'query' => $query,
    'limit' => false, // default
]);

The 100 limit on the query is ignored. The pagination will range over all records in the table. This is the same as the current (and as @dynasource said, unexpected) behavior when the developer has gone out of their way to set a limit on the query before using it in the ActiveDataProvider.

The end user would see as many pages of posts as needed to show all rows in the database table.

Example 2

$query = Posts::find()->orderBy('created_at DESC')->limit(100);
$dataProvider = new ActiveDataProvider([
    'query' => $query,
    'limit' => true, // respect the query limit
]);

The 100 limit on the query is respected. The prepareTotalCount() method in ActiveDataProvider will reflect 100 at most.

The end user would see at most 10 pages of 10 posts each.

Example 3

$query = Posts::find()->orderBy('created_at DESC')->limit(100);
$dataProvider = new ActiveDataProvider([
    'query' => $query,
    'limit' => 120, // explicit limit
]);

The 100 limit on the query is ignored, but a 120 limit is imposed. The prepareTotalCount() method in ActiveDataProvider will reflect 120 at most.

The end user would see at most 12 pages of 10 posts each.

crimson-med commented 5 years ago

This was supposed to be integrated in Milestone 2.1.1 but it hasn't been? I'm trying to display only 20 results paged with 5 items and it isn't working.

samdark commented 5 years ago

There was no 2.1.1 release. It's moved to Yii 3.0.

samdark commented 2 months ago

Valid use case.

vjik commented 2 months ago

Note. When passed data reader with limit to grid view with allowed sorting, need throw logic exception.

samdark commented 2 months ago

Closing, since there's no need to implement it in this package. It is yiisoft/data entirely.

samdark commented 2 months ago

Still need a check. If data reader has limit then sorting should be disabled.

vjik commented 1 month ago

Still need a check. If data reader has limit then sorting should be disabled.

Maybe it's better to throw logic exception, so developer should change own code?

samdark commented 1 month ago

Grid without sorting does make sense so I don't think throwing an exception is a good idea.

vjik commented 1 month ago

Grid without sorting does make sense so I don't think throwing an exception is a good idea.

Throw exception when data reader has limit and sorting enabled in grid view only. When sorting disabled don't throw exception.

samdark commented 1 month ago

Makes sense.