catfan / Medoo

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

function tableQuote($table) does not work with MySQL #759

Closed shinsenter closed 5 years ago

shinsenter commented 6 years ago

Function tableQuote($table) does not work with MySQL.

 [PDOException]
SQLSTATE[42000]: Syntax error or access violation: 1064 
You have an error in your SQL syntax; check the manual 
that corresponds to your MySQL server version for the right syntax 
to use near 'FROM "my_table" LIMIT 1' at line 1
catfan commented 6 years ago

How do you use it? tableQuote() is a protected function.

And what's the debug() output?

LucaRainone commented 6 years ago

I had the same issue. The problem happens when we configure Medoo with dsn option and we try to use magic methods like insert or update.

In fact, seeing the code, we have the init command that allows you to use the double quote for tableQuote 'SET SQL_MODE=ANSI_QUOTES'; here

https://github.com/catfan/Medoo/blob/master/src/Medoo.php#L125

but this command is unreachable if there is the dsn option, regardless the presence of database_type option.

I made 2 pull request. https://github.com/catfan/Medoo/pull/784 https://github.com/catfan/Medoo/pull/785

The first one add the init commands also in dsn builder section BUT only if database_type is set. Reading the doc, I think that it's absolutely coherent with it. This solve this issue.

The second is more invasive. Because the above command change the query syntax (https://dev.mysql.com/doc/refman/8.0/en/sql-mode.html#sqlmode_ansi_quotes)

With ANSI_QUOTES enabled, you cannot use double quotation marks to quote literal strings because they are interpreted as identifiers.

I try to respect the default behavior of mysql engine. So I need to configure the tableQuote and tableColumn character if the driver is mysql. Because there are only few issues for this problem, in the pull request I made the new feature only for dsn options but it should be a global behavior I think. However there are no tests and I don't know how many things can breaks this "feature".

catfan commented 6 years ago

While you are using Medoo, you don't have to care about the quotation. No matter how Medoo works internally.

It also provided a placeholder and quotation syntax, a simple way for handle quoting problem. Never need to thinking about double quote or single quote identifier.

LucaRainone commented 6 years ago

Code to reproduce:

require 'vendor/autoload.php';
use Medoo\Medoo;

// initialize with dsn param
$db = new Medoo([
        'dsn'      => [
            'driver'  => "mysql",
            'host'    => "localhost", // use your host
            'port'    => 3306
        ],
        'database_type' => 'mysql',
        'username' => "root", // use your credential
        'password' => "root",  // use your credential
]);
$stmt = $db->insert("mytable", ['mycol' => 1]);
var_dump($stmt->errorInfo()[2]);

expected result (in case of no database previous selection):

'No database selected'

actual result:

You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near '"mytable" ("mycol") VALUES (1)' at line 1

catfan commented 6 years ago

@LucaRainone

DSN option is for special database with special driver and connection option. If you are using MySQL database, you don't have to consider this.

LucaRainone commented 6 years ago

In doc:

You can also use customized DSN to connect the database that Medoo didn't supported by default, especially for some new database requiring special DSN parameter for database connection, or if you want to add more DSN parameters value for connection than original one.

There is no explicity deny for using this with MySQL. (And I used this because I've already a shared array for dsn connection: I could not imagine all these implications).

But anyway the main problem, for me, is that Medoo change the sql_mode.

If the sql_mode is set as TRADITIONAL by default, as I had, the MySQL does not allows dirty insert or update (missing fields without default value, zero time dates and so on). I totally miss all these restrictions using Medoo, and I don't realized it (because I don't should care about it works internally).

A workaround is to do on app init:

$db->query("SET sql_mode='TRADITIONAL,ANSI_QUOTES'");

My opinion: I encountered a lot of problems with something of simple (adding custom query with double quotes for string before realizing the problem) and all because the change of sql_mode. I don't know if there are iron reasons for this, but if not, please consider of don't do it.

catfan commented 6 years ago

We have provided quotation syntax for query() for handling the quote problem, so you don't have to add quote character for custom query.

We don't suggest that to use custom DSN option if the default option works already, especially MySQL. It's for special database connection.

We set the SQL_MODE for compatibility problem. We not only support MySQL, but also others database. That make MySQL using standard SQL syntax and behaviour.

LucaRainone commented 6 years ago

I understand the reasons and the legitim approach.

Maybe the first pull request is still valid because it allows you to build a Medoo instance with dsn attribute exactly like the primary way, if and only if the "database_type" option is set (as doc says: allowed but not mandatory in this case). Otherwise you can close both pull request

Thanks for clarification.

catfan commented 5 years ago

@LucaRainone Yes, you are right. I made a new commit with this fix. 462e7a8

Rarst commented 3 years ago

I seem to have ran into this problem with PHP MySQL Engine.

It's meant to emulate MySQL for testing, but it doesn't seem to have support for ANSI_QUOTES so it does not correctly parse queries that Medoo generates with double quotes around identifiers.