catfan / Medoo

The lightweight PHP database framework to accelerate the development.
https://medoo.in
MIT License
4.83k stars 1.15k forks source link

Unable to execute a query from a remote server such as Planetscale #1044

Open jcchikikomori opened 2 years ago

jcchikikomori commented 2 years ago

Information

Describe the Problem Uncaught PDOException near either column name or FROM

Detail Code The detail code you are using causes the problem.

// Write your PHP code here
use Medoo\Medoo;

public function connect_database(): Medoo
{
        $database_properties = [
          'type' => $_ENV['DB_TYPE'] ?? 'mysql',
          'database' => $_ENV['DB_NAME'],
          'host' => $_ENV['DB_HOST'],
          'username' => $_ENV['DB_USER'],
          'password' => $_ENV['DB_PASS'],
          'charset' => $_ENV['DB_CHARSET'] ?? null,
          'port' => $_ENV['DB_PORT'] ?? null, // if defined then use, else default
          'option' => [
            PDO::ATTR_CASE => PDO::CASE_NATURAL,
            PDO::MYSQL_ATTR_SSL_CA => '/etc/ssl/cert.pem' // using planetscale as Database server
          ],
          'logging' => true,
        ];
        return new Medoo($database_properties);
}

var_dump($this->connect_database()->debug()->select("user_types", "*")); die();

Expected output Working query with var_dump()

array(2) { [0]=> array(3) { ["id"]=> int(0) ["user_type"]=> string(5) "admin" ["type_desc"]=> string(14) "Administrators" } [1]=> array(3) { ["id"]=> int(1) ["user_type"]=> string(4) "user" ["type_desc"]=> string(5) "Users" } }

Notes Working in native implementation

use PDO;

$dsn = "mysql:host={$_ENV['DB_HOST']};dbname={$_ENV['DB_NAME']}";
$options = array(PDO::MYSQL_ATTR_SSL_CA => "/etc/ssl/cert.pem");
$pdo = new PDO($dsn, $_ENV["DB_USER"], $_ENV["DB_PASS"], $options);
$query = $pdo->prepare('SELECT * FROM `user_types`');
$query->execute();
$result = $query->fetchAll(PDO::FETCH_ASSOC);
var_dump($result); die();
jcchikikomori commented 2 years ago

Error Shown

Fatal error: Uncaught PDOException: SQLSTATE[HY000]: General error: 1105 syntax error at position 49 near 'user_types' in /Users/redacted/www/hello-php/vendor/catfan/medoo/src/Medoo.php:563 Stack trace: #0 /Users/redacted/www/hello-php/vendor/catfan/medoo/src/Medoo.php(563): PDOStatement->execute() #1 /Users/redacted/www/hello-php/vendor/catfan/medoo/src/Medoo.php(1595): Medoo\Medoo->exec(Object(PDOStatement), Array) #2 /Users/redacted/www/hello-php/classes/Registration.php(95): Medoo\Medoo->select('user_types', Array) #3 /Users/redacted/www/hello-php/register.php(24): classes\Registration->getUserTypes() #4 {main} thrown in /Users/redacted/www/hello-php/vendor/catfan/medoo/src/Medoo.php on line 563
catfan commented 2 years ago

Are your $_ENV['DB_TYPE'] configured as mysql correctly?

What the output of $database->info()['dsn']; and the result of $database->debug()->select("user_types", "*")?

Also remove charset, port, PDO::ATTR_CASE connection option to debug.

chriscct7 commented 2 years ago

We're running into the same issue on PlanetScale.

The DB type is set to MySQL (hardcoded), and we don't use custom charsets, ports or ATTR cases.

When we run debug() and ask Medoo to output the SQL it thinks it should run, the SQL statement outputted is correct and runs fine if we execute it manually, but for some reason, that SQL statement is not the one Medoo is actually running.

We're able to replicate this with an extremely simple test case:

global $container;
$container['db'] = function () {
    return new Medoo(
        [
            'database_type' => 'mysql',
            'database_name' => DB_NAME,
            'server'        => DB_HOST,
            'username'      => DB_USER,
            'password'      => DB_PASSWORD,
            'option'        => array(
                \PDO::ATTR_ERRMODE       => \PDO::ERRMODE_EXCEPTION,
                \PDO::MYSQL_ATTR_SSL_CA   => dirname(__FILE__) . '/cacert.pem'
            )
        ]
    );
};
$app = $container['db']->select( 'test_table',['id'], [ "LIMIT" => 1 ] );

where the test_table has 2 columns, an autoincrementing ID column and a VARCHAR value column

We also get a syntax error like the previous reporter.

We've confirmed the DB variables are correctly set (though we wouldn't get the syntax error but rather a connection failed error if they weren't) and we did the same for the cacert.pem to ensure it's in the right place (and again it would also trigger a different error if it was in the wrong place).

Note, if it helps, PlanetScale offers a free db plan, and we're able to replicate this issue on that plan as well.

chriscct7 commented 2 years ago

Notably, if I run

$id = $container['db']->query("SELECT `id` FROM `test_table` LIMIT 1;")->fetchAll();

The result works with no error.

chriscct7 commented 2 years ago

Ok, progress to report!

I have figured out that with the following modifications, the library starts working on PlanetScale's databases:

In the tableQuote function, changing it to return the column in a tick instead of in double quotes:

return '`' . $this->prefix . $table . '`';

and in the columnQuote function, doing the same:

return strpos($column, '.') !== false ?
                '`' . $this->prefix . str_replace('.', '"."', $column) . '`' :
                '`' . $column . '`';

This has to be related to the handling of the MySQL ANSI_QUOTES

chriscct7 commented 2 years ago

Ok, so this seems to be done because of #578, where it discusses the use of double quotes instead of backticks to adhere to the SQL-92 standard. It seems that PlanetScale/Vitess might not support setting SQL_MODE = ANSI_QUOTES, which the library tries to do. Resultingly, the queries fail.

chriscct7 commented 2 years ago

It looks like Vitess doesn't support alternative SQL_MODES per doc here

@catfan would it be okay if we submit a pr to allow for an option that would let PlanetScale users (and similar MySQL compatible database types that don't support the changing of SQL_MODE) to use backticks instead of double quotes for these two cases?

Also, are there other places we need to account for the double quotes other than just the two places mentioned above if SQL_MODE can't be altered to ANSI_QUOTES?

chriscct7 commented 2 years ago

Note, the lack of ANSI_QUOTES support is a known issue for Vitess: https://github.com/vitessio/vitess/issues/7769 which PlanetScale runs

catfan commented 2 years ago

@chriscct7 Thank you for your inspection. I think it's good to call Vitess support ANSI_QUOTES to work like MySQL as possible. Medoo is using this standard quotation for all supported databases.

For your case, you can just fork the Medoo project and change the quotation alone to make it work until Vitess support ANSI_QUOTES officially.

orware commented 2 years ago

@catfan,

I've put together the associated PR#1061 above in case it might be considered as a potential solution for this, considering that the likelihood of ANSI_QUOTES support being added on the Vitess side in the near future is fairly slim and this would be a workable solution that shouldn't affect existing users, but would provide flexibility for PlanetScale users (or any other database that might need this workaround) to still make use of Medoo easily.

The usage is quite simple and after instantiating a new Medoo instance you would simply add a call to setIdentifierDelimiter() and pass in the value you want to utilize instead of the default double quote value.

chriscct7 commented 2 years ago

We've tested the PR orware submitted above and it works fantastically and solves #1044 when used.