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

PDO integration is rough #322

Closed
athoune opened this issue Sep 14, 2020 · 13 comments
Closed

PDO integration is rough #322

athoune opened this issue Sep 14, 2020 · 13 comments

Comments

@athoune
Copy link

athoune commented Sep 14, 2020

I'm trying to use Mysql PDO connector (with a Mariadb 10.1), with XHGui 0.13.0 :

  • The lazy table creation never throw error in src/xhgui/vendor/perftools/xhgui-collector/src/Xhgui/Saver/Pdo.php
  • Table name is escaped with ", and mysql cli doesn't like it, ` seems to be more standard
  • The id column is not specified enough : ERROR 1170 (42000): BLOB/TEXT column 'id' used in key specification without a key length
@glensc
Copy link
Contributor

glensc commented Sep 14, 2020

Now that xhgui-collector is imported back to this project:

can perhaps use doctrine or cakephp/database abstractions for database.

altho these indicated by you could be just fixed to be compatible with sqlite/mysql/pgsql simple changes.

@glensc
Copy link
Contributor

glensc commented Sep 14, 2020

The lazy table creation never throw error in src/xhgui/vendor/perftools/xhgui-collector/src/Xhgui/Saver/Pdo.php

could you please elaborate what that statement means?

@glensc
Copy link
Contributor

glensc commented Sep 14, 2020

Table name is escaped with ", and mysql cli doesn't like it, ` seems to be more standard

seems pgsql/sqlite accept ", MySQL accepts `, so the only portable way without too much abstraction is to remove escaping at all, i.e use bare words.

some alternative solutions is to let it be configurable

ref:

Quoting

In MySQL and SQLite you use backticks for identifiers. PostgreSQL uses single quotes. SQlite can also use single quotes here if the result is unambigious, but I would strongly suggest to avoid that.

@glensc
Copy link
Contributor

glensc commented Sep 14, 2020

The id column is not specified enough : ERROR 1170 (42000): BLOB/TEXT column 'id' used in key specification without a key length

changed id column to match MongoId length: 24 characters:

NOTE: this will break on 2038/2106 year (depending on cpu bits). likely fix is to increase column length to be 25 characters. but deal with the problem when it's due :)

@athoune
Copy link
Author

athoune commented Sep 14, 2020

The lazy table creation never throw error in src/xhgui/vendor/perftools/xhgui-collector/src/Xhgui/Saver/Pdo.php
could you please elaborate what that statement means?

My ugly debug is :

$ok = $pdo->exec(sprintf(self::TABLE_DDL, $table));
error_log("Create table : $ok");

Throwing an exception when $ok is false can be more simple.

@athoune
Copy link
Author

athoune commented Sep 14, 2020

I just inject my first xhprof trace to Mysql with the 322-pdo branch !

@glensc
Copy link
Contributor

glensc commented Sep 14, 2020

The lazy table creation never throw error in src/xhgui/vendor/perftools/xhgui-collector/src/Xhgui/Saver/Pdo.php

so your statement means table creation errors are not checked!

@glensc
Copy link
Contributor

glensc commented Sep 15, 2020

Besides table name, GET column also needs to be escaped for MySQL (at least for 5.7)

So, omitting escaping is out of the question. Still need to abstract db layer

@glensc
Copy link
Contributor

glensc commented Sep 15, 2020

also, pgsql fails with:

1) XHGui\Test\Controller\RunTest::testIndexEmpty
PDOException: SQLSTATE[42601]: Syntax error: 7 ERROR:  LIMIT #,# syntax is not supported
LINE 20:           LIMIT 0, 25

so, I think currently only sqlite was really supported?

@athoune
Copy link
Author

athoune commented Sep 16, 2020

Ok, you are speaking about this part of Doctrine : https://github.com/doctrine/dbal/ not the ORM part.

@glensc
Copy link
Contributor

glensc commented Sep 16, 2020

@athoune can you find compatible quoting for table names and column names that are valid for all common pdo modules:

  • mysql
  • pgsql
  • sqlite

i think no such exist, and can't omit quoting as GET is reserved keyword in mysql?

@athoune
Copy link
Author

athoune commented Sep 22, 2020

The Doctrine's DBAL handles all the little differences between databases :

I don't know what is the simplest way to do, trusting Doctrine job for true universal DB handling, or tiny handcrafted code.

@glensc
Copy link
Contributor

glensc commented Sep 24, 2020

@glensc glensc closed this as completed Sep 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants