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

Relation models number limits #14062

Closed
dryu opened this issue Apr 26, 2017 · 5 comments
Closed

Relation models number limits #14062

dryu opened this issue Apr 26, 2017 · 5 comments

Comments

@dryu
Copy link

dryu commented Apr 26, 2017

Ok, I know those limits were discussed in depth in #10305 and #10371, and I agree with comments that having too much of IDs inside IN() statement is a bad idea, however, I don't agree with the conclusion to not do anything.

The concern raised was that building up lots of models takes lots of memory and there are risks of having memory overflow exceptions. But if we don't do that, Yii2 fails miserably when trying to pull models with a total number more than the underlying database allows inside IN() statement. So it either always fails because of the database limitation not accounted properly in Yii2, or sometimes fails if memory thresholds for PHP are reached. Why was it decided for the former instead of a later?

As of now I just don't see any legal workarounds against that issue. I have a huge relational hierarchy defined in my application. The logic pulls 1 root model (e.g. a blog post), and then it tries to pull lots of other related things, such as comments, authors, etc, via relations. On a very rare occasion, the number of comments goes beyond 1000 at which point the logic fails to pull models related to those comments and linked via a comment primary key, and in the end, the blog post cannot be displayed. I cannot batch the comments, because those are being pulled indirectly (through relations), rather than directly, thus they are always resolved with all().

$post = BlogPost::find()->with('comments.time')->one();

I don't want to manage comments separately from the blog post, e.g. I don't want that:

$comments = $post->getComments()->all();

Because in the above case comments are not present from within the parent model $post, and have to be resolved again if they are accessed again.

I want to handle comments and all models related to comments like that:

$comments = $post->comments;

And I want all comments on the same page, no pagination. This is a design decision, and we can discuss in length whether it is correct or not, but right now, I just don't see how to do that in Yii2 without hacks.

I personally believe it is better to hit potential memory thresholds when trying to construct thousands of models, rather than allow a database exception from unaccounted database limitations.

Oh, and speaking of hacks, I had to change ActiveRelationTrait class to break IN() statements in batches of 1000 items to workaround the issue (in case anyone still wants to have that functionality):

    private function filterByModels($models)
    {
        $attributes = array_keys($this->link);

        $attributes = $this->prefixKeyColumns($attributes);

        $values = [];
        if (count($attributes) === 1) {
            // single key
            $attribute = reset($this->link);
            foreach ($models as $model) {
                if (($value = $model[$attribute]) !== null) {
                    if (is_array($value)) {
                        $values = array_merge($values, $value);
                    } else {
                        $values[] = $value;
                    }
                }
            }
        } else {
            // composite keys

            // ensure keys of $this->link are prefixed the same way as $attributes
            $prefixedLink = array_combine(
                $attributes,
                array_values($this->link)
            );
            foreach ($models as $model) {
                $v = [];
                foreach ($prefixedLink as $attribute => $link) {
                    $v[$attribute] = $model[$link];
                }
                $values[] = $v;
            }
        }
        // MAX 1000 items fix -- dryu
	    if (!empty($values)) {
		    $values = array_unique($values, SORT_REGULAR);
		    $condition = ["or"];
		    for ($i=0, $n=1000, $c = count($values); $i<$c; $i+=$n) {
			    $condition[] = ['in', $attributes, array_slice($values, $i, $n)];
		    }
		    $this->andWhere($condition);
	    } else {
		    $this->andWhere(['in', $attributes, array_unique($values, SORT_REGULAR)]);
	    }
	    //$this->andWhere(['in', $attributes, array_unique($values, SORT_REGULAR)]);
    }

The above results in related models resolved via a single query where model keys are split by 1000 items and glued together via OR.

I actually wouldn't mind supporting this change in a legal way (like extending some base class and then passing my custom class to a configuration array), but there are no hooks available.

What steps will reproduce the problem?

$post = BlogPost::find()->with('comments.time')->one();

What is the expected result?

All 1000+ comments to be pulled and available via $blog->comments.

What do you get instead?

2017-04-26 12:50:16 [10.252.57.75][xxxx][hgoh96v4ifqbpicsm5ejpa30d7][error][yii\db\Exception] PDOException: SQLSTATE[HY000]: General error: 1795 OCIStmtExecute: ORA-01795: maximum number of expressions in a list is 1000 
(ext\pdo_oci\oci_statement.c:159) in D:\qtool\vendor\yiisoft\yii2\db\Command.php:900
Stack trace:
#0 D:\qtool\vendor\yiisoft\yii2\db\Command.php(900): PDOStatement->execute()
#1 D:\qtool\vendor\yiisoft\yii2\db\Command.php(362): yii\db\Command->queryInternal('fetchAll', NULL)
#2 D:\qtool\vendor\yiisoft\yii2\db\Query.php(210): yii\db\Command->queryAll()
#3 D:\qtool\vendor\yiisoft\yii2\db\ActiveQuery.php(133): yii\db\Query->all(NULL)
#4 D:\qtool\vendor\yiisoft\yii2\db\ActiveRelationTrait.php(239): yii\db\ActiveQuery->all()
#5 D:\qtool\vendor\yiisoft\yii2\db\ActiveQueryTrait.php(173): yii\db\ActiveQuery->populateRelation('time', Array)
#6 D:\qtool\vendor\yiisoft\yii2\db\ActiveQuery.php(215): yii\db\ActiveQuery->findWith(Array, Array)
#7 D:\qtool\vendor\yiisoft\yii2\db\Query.php(211): yii\db\ActiveQuery->populate(Array)
#8 D:\qtool\vendor\yiisoft\yii2\db\ActiveQuery.php(133): yii\db\Query->all(NULL)
#9 D:\qtool\vendor\yiisoft\yii2\db\ActiveRelationTrait.php(239): yii\db\ActiveQuery->all()
#10 D:\qtool\vendor\yiisoft\yii2\db\ActiveQueryTrait.php(173): yii\db\ActiveQuery->populateRelation('comments', Array)
#11 D:\qtool\vendor\yiisoft\yii2\db\ActiveQuery.php(215): yii\db\ActiveQuery->findWith(Array, Array)
#12 D:\qtool\vendor\yiisoft\yii2\db\ActiveQuery.php(297): yii\db\ActiveQuery->populate(Array)
#13 D:\qtool\controllers\ViewController.php(130): yii\db\ActiveQuery->one()

Additional info

Q A
Yii version 2.0.10
PHP version 7.0
Operating system Windows
@SilverFire
Copy link
Member

@dryu thank you for the detailed description of your POV.

I personally believe it is better to hit potential memory thresholds when trying to construct thousands of models, rather than allow a database exception from unaccounted database limitations.

This sounds pretty reasonable, I agree with you.

I'll think what we can do to solve this problem

@samdark samdark added this to the 2.0.14 milestone Apr 26, 2017
@cebe
Copy link
Member

cebe commented Apr 26, 2017

$post = BlogPost::find()->with('comments')->one();

How do you manage to run into IN limitation with this query?
I would expect the queries for that to be like:

SELECT * FROM blog_post LIMIT 1;
SELECT * FROM comment WHERE blog_post_id=42;

same for $post->comments; which should result in the latter one, there should not be any IN condition involved.

@dryu
Copy link
Author

dryu commented Apr 26, 2017

The problems start to appear when I reference a model relative to comments, e.g. in the example above it is comments.time. When Yii2 tries to retrieve that, it runs the query:

SELECT * FROM CommentsTime WHERE comment_id IN (...)

And there goes all ids of all comments retrieved prior to that.

@SilverFire
Copy link
Member

See #14065

@SilverFire
Copy link
Member

Fixed in f6a4f3c

@SilverFire SilverFire self-assigned this May 1, 2017
@SilverFire SilverFire modified the milestones: 2.0.12, 2.0.14 May 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants