yiisoft / yii2

Yii 2: The Fast, Secure and Professional PHP Framework
http://www.yiiframework.com
BSD 3-Clause "New" or "Revised" License
14.24k stars 6.91k forks source link

Yii opens db connection even when query result got from yii's cache #13749

Closed Stern87 closed 4 years ago

Stern87 commented 7 years ago

Hey, guys! I have noticed the next problem: When I am trying to find something with ActiveRecord with "where" as array - the yii opens db connection and then gets the result from cache system. But when I'm using "where" as a string - problem disappears:

$alias = "welcome";
$model = StaticPages::getDb()->cache(
    function ($db) use ($alias) {
        return StaticPages::find()->where("alias = 'test'")->one();
    },
    3600
);

It looks like quoteValue function opens db connection.

What steps will reproduce the problem?

A piece of component's config:

<?php
return [
    // ...
    components' => [
        'db'      => [
            'class'               => 'yii\db\Connection',
            'dsn'                 => 'mysql:host=localhost;dbname=web',
            'username'            => 'web',
            'password'            => '1234567',
            'charset'             => 'utf8',
            'enableSchemaCache'   => true,
            'schemaCacheDuration' => 3600,
            'schemaCache'         => 'cache',
        ],
        'cache'   => [
            'class'        => 'yii\caching\MemCache',
            'useMemcached' => true,
            'servers'      => [
                'main' => [
                    'host'   => 'localhost',
                    'port'   => 11211,
                    'weight' => 100,
                ],
            ],
        ],
    // ...
];

Example:

$alias = "welcome";
$model = StaticPages::getDb()->cache(
    function ($db) use ($alias) {
        return StaticPages::find()->where(['alias' => $alias, 'language' => Yii::$app->language])->one();
    },
    3600
);

What is the expected result?

Yii mustn't open db connection.

What do you get instead?

Yii opens db connection.

Additional info

Q A
Yii version 2.0.11.2
PHP version 5.4.16
Operating system CentOS 7.3
samdark commented 7 years ago

Indeed quoteValue opens connection.

cebe commented 7 years ago

It does that because it needs PDO for quoting the value.

SilverFire commented 7 years ago

Agreed with @cebe, behavior is correct. In case you have slaves, connection will be opened to the slave DBMS instance.

Stern87 commented 7 years ago

Did you see in my config slave connections? I did not. And there are no slave configs. So, behavior is incorrect. Make a try to pay more attention to yii2 framework development. Right now you are pushing to use "where" as string instead of array for case like above, maybe because of laziness?

samdark commented 7 years ago

I don't see why connection is opened in case of existence of cached value.

cebe commented 7 years ago

This issue is not easy to fix but it could be possible to fix it. This depends on how quote is implemented on the PDO drivers. If the driver needs a database connection to quote, it is not possible to fix it and not a Yii issue but a PDO one. If PDO can quote without DB connection we might be able to instantiate a PDO instance in this case without opening a connection. This will be a bit complicated because of master/slave implementation but should be still possible.

cebe commented 7 years ago

We may also be able to delay the quoting to be done after caching checks, needs to be checked.

SamMousa commented 7 years ago

@cebe You cannot possibly quote without opening a database connection. Quoting depends on the server settings and connection settings, like character set. Which are not known before connecting.

SamMousa commented 7 years ago

If you want to check the cache then support for that needs to be added to the Query layer; which is a big change. Also note that this would break with parameter binding since those bindings might be changed.

samdark commented 7 years ago

That's implementation difficulties. It's not making current behavior correct.

SamMousa commented 7 years ago

True, but it is an indicator that this is not about query caching, but more about model caching. Also it would replace the cost of setting up the database connection with serializing the query object without database connection (to get the cache key). I'm all in favor of supporting this, just saying that it might be a lot of work for a little gain. Especially since most requests will hit the database at some point. For this use case it seems view / data caching is more appropriate.

titanproger commented 7 years ago

Hello! I use yii 2.0.12 with php7.1 and postgress PDO. And I have exactly same issue. I solved that by this way.

As i see \yii\db\Connection::quoteValue used mainly by \yii\db\Command::getRawSql and only for log and cache purposes

So i just create my own db connection class and rewrite quoteValue like this

namespace common\components;
use yii\db\Connection;
class TDbConnectionCustomQuote extends  Connection {
    /** @inheritdoc */
    public function quoteValue($value)
    {
        // quoteValue needs open connection
        // it is bad for caching
        // so make own quote

        // it is secure enough, as for real sql prepare is used.
        // quoteValue used only for log and cache

        // code here is from \yii\db\Schema::quoteValue
        if (!is_string($value))
            return $value;

        return "'" . addcslashes(str_replace("'", "''", $value), "\000\n\r\\\032") . "'";
    }
}

Another approach i think is create own implementation of \yii\db\Command::getRawSql with own quoteValue() function. It is slightly more secure. Hope it helps some one. please tell me if you see i am wrong somewhere.

SamMousa commented 7 years ago

I was thinking some more about this and we could use something else than getRawSql() for the query key of course... Instead we can simply use getSql and serialize the params ourselves (for a cache key there is no DB specific serialization we care about) so we could simply json_encode.

andrei-livadariu commented 5 years ago

I found that there is one more place in the code where a database connection is opened even if there is a cache in front of it: the supportsFractionalSeconds method in the \mysql\QueryBuilder class.

In my own code I added an override for this class and method that always returns true (not the cleanest solution). I suggest exposing this property as a config parameter somehow, and only checking the database if the config parameter is not provided.

lumenpink commented 5 years ago

Hello @andrei-livadariu ! I am having exactly the same issue. How did you managed the method override? I'm not that good in OOP to manage that by myself.

sartor commented 4 years ago

I have another issue, but problem same. I have master and slave dbs. Some of my requests uses only master connection, but when quoteValue() function is called - it opens slave connection (tooks useless 10ms of time). I have highload project and this useless connection opens taking big amount of time.

Problem in db\Schema class:

    public function quoteValue($str)
    {
        if (!is_string($str)) {
            return $str;
        }

        if (($value = $this->db->getSlavePdo()->quote($str)) !== false) {
            return $value;
        }

        // the driver doesn't support quote (e.g. oci)
        return "'" . addcslashes(str_replace("'", "''", $str), "\000\n\r\\\032") . "'";
    }
samdark commented 4 years ago

@sartor that's separate issue. Let's discuss it separately.

sartor commented 4 years ago

Ok, I'll create new issue and maybe pull request with solution

shushenghong commented 4 years ago

I was thinking some more about this and we could use something else than getRawSql() for the query key of course... Instead we can simply use getSql and serialize the params ourselves (for a cache key there is no DB specific serialization we care about) so we could simply json_encode.

any progress?