ZF-Commons / ZfcUserDoctrineORM

Doctrine2 ORM storage adapter for ZfcUser.
BSD 3-Clause "New" or "Revised" License
89 stars 92 forks source link

"user" is a restricted keyword in postgres #77

Closed Danielss89 closed 10 years ago

Danielss89 commented 10 years ago

As headline says, "user" is a restricted keyword in postgres, and that's what we are using for table name. If we add backticks it works on web, but in CLI, when running ./vendor/bin/doctrine-module orm:validate-schema it says

[Mapping]  OK - The mapping files are correct.
[Database] FAIL - The database schema is not in sync with the current mapping file

Changing the table name to users fixes everything, but that would be a major BC break.

ekancepts commented 10 years ago

Just changing 'user' to 'users' will not fix the issue i think.

Will do some trail and errors.

ekancepts commented 10 years ago

Changed table name in ~/zfc-user-doctrine-orm/config/xml/zfcuserdoctrineorm/ZfcUserDoctrineORM.Entity.User.dcm.xml from user to users

Then in terminal

$ vendor/bin/doctrine-module orm:schema-tool:create
ATTENTION: This operation should not be executed in a production environment.

Creating database schema...
Database schema created successfully!
$ vendor/bin/doctrine-module orm:schema-tool:create
ATTENTION: This operation should not be executed in a production environment.

Creating database schema...
Database schema created successfully!
$ vendor/bin/doctrine-module orm:validate-schema
[Mapping]  OK - The mapping files are correct.
[Database] OK - The database schema is in sync with the mapping files.

Till now there was no problem. Was able to create user using zfcuser, doctrineorm and PostgreSQL.

@Danielss89 : What 'major BC break' were you referring to? If you think this solution is good then i will send PR.

This solves ZF-Commons/ZfcUser#435

Danielss89 commented 10 years ago

If we change the table name to users instead of user, then when someone upgrades, it will break their app, as the table name has changed. It must be possible to escape in a way so that everything works correct. @ocramius ping?

Ocramius commented 10 years ago

The change is a bc break - instead, quote the name by using backticks.

Ocramius commented 10 years ago

@ekancepts eventially, you can send a PR, but it would be rescheduled for the next major

Danielss89 commented 10 years ago

@Ocramius if i add backticks the database validation fails "[Database] FAIL - The database schema is not in sync with the current mapping file" even though it is up2date

Ocramius commented 10 years ago

@Danielss89 what does the SQL dump say?

Danielss89 commented 10 years ago

@Ocramius https://gist.github.com/Danielss89/fff4eb885891cf367fbc

ekancepts commented 10 years ago

There should be 'namespace' like concept in DB too ... or is it already there?

Ocramius commented 10 years ago

@ekancepts postgresql has databases and schemas for that.

@Danielss89 weird - that's indeed an annoying issue, but it shouldn't affect first installs. The problem is that "user" and user are different in pgsql, as it seems. Does that persist even after removing the table?

Ocramius commented 10 years ago

May need some infos from @deeky666

deeky666 commented 10 years ago

@Ocramius I looked into this and realized that reverse engineering is not working as expected for tables that use reserved keywords as name. AbstractSchemaManager::createSchema() fetches all table names, then lists table details such as columns, indexes etc. for each table. Unfortunately it tries to fetch the table details for "user" instead of user which does not return any results for columns, indexes etc, therefore the user table object is initialized "empty" and produces those ALTER TABLE statements after comparison. This is DBAL bug and has to be addressed there.

deeky666 commented 10 years ago

I think this commit causes the bug: https://github.com/doctrine/dbal/commit/83fe296de44ef3e2f04f2342021f656a797787ec

deeky666 commented 10 years ago

@Ocramius @Danielss89 patch provided in https://github.com/doctrine/dbal/pull/572

Danielss89 commented 10 years ago

@ekancepts can you confirm this is working with the merge of doctrine/dbal#572 ?

ekancepts commented 10 years ago

Hi @Danielss89 ... sorry for the delay ... been setting up my new system ... will test in a day and update the same.

Danielss89 commented 10 years ago

I'm going to close this for now. Reopen if issue persists.

tdutrion commented 9 years ago

I'd like to reopen the issue...

With doctrine dbal up to date:

"name": "doctrine/dbal",
"version": "v2.5.1",

PostgreSQL is running in a docker container on the default postgres image from docker hub (no impact here): PostgreSQL 9.4.1 on x86_64-unknown-linux-gnu, compiled by gcc (Debian 4.7.2-5) 4.7.2, 64-bit


Doctrine\DBAL\Exception\InvalidFieldNameException

File:

/code/vendor/doctrine/dbal/lib/Doctrine/DBAL/Driver/AbstractPostgreSQLDriver.php:72

Message:

An exception occurred while executing 'SELECT t0.username AS username_1, t0.email AS email_2, t0.display_name AS display_name_3, t0.password AS password_4, t0.state AS state_5, t0.user_id AS user_id_6 FROM user t0 WHERE t0.email = ? LIMIT 1' with params ["test@email.com"]:

SQLSTATE[42703]: Undefined column: 7 ERROR:  column t0.username does not exist
LINE 1: SELECT t0.username AS username_1, t0.email AS email_2, t0.di...
               ^

Running the sql query with double quotes surrounding the table name solve the problem (directly in PgAdmin for test purpose).