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

Quote table names that also are reserved keywords #334

Conversation

stephen-lewis
Copy link
Contributor

@stephen-lewis stephen-lewis commented Jan 2, 2020

Technically a duplicate of PR #292 but with tests included.

Essentially, purging the database in DELETE mode prior to fixture load fails if any fixture entities use tables names with reserved words (e.g. 'User' in Postgres, or in the test case 'Group' in sqlite). The change makes ORMPurger->getTableName() use the same method as AbstractPlatform::getTruncateTableSQL() uses to get the table name (quoted as necessary).

Test case uses sqlite reserved word 'Group' as an example and checks returned table name is quoted correctly.

Closes #292

@greg0ire

This comment has been minimized.

@greg0ire greg0ire closed this Jan 4, 2020
@greg0ire greg0ire reopened this Jan 4, 2020
@greg0ire
Copy link
Member

greg0ire commented Jan 4, 2020

#333 is not merged up yet, hence why the build is not fixed. Are you sure you should target master though? Is there a BC-break in your PR?

EDIT it's a patch so I guess/hope not, let's retarget

@greg0ire greg0ire changed the base branch from master to 1.4.x January 4, 2020 15:42
@greg0ire greg0ire closed this Jan 4, 2020
@greg0ire greg0ire reopened this Jan 4, 2020
@greg0ire greg0ire force-pushed the bugfix/quote_reserved_table_names_in_purger_delete_mode branch 2 times, most recently from cd113d0 to 25a09a8 Compare January 4, 2020 16:11
@greg0ire greg0ire requested a review from a team January 4, 2020 16:11
Essentially, purging the database in DELETE mode prior to fixture load
fails if any fixture entities use tables names with reserved words (e.g.
'User' in Postgres, or in the test case 'Group' in sqlite). The change
makes ORMPurger->getTableName() use the same method as
AbstractPlatform::getTruncateTableSQL() uses to get the table name
(quoted as necessary).
Test case uses sqlite reserved word 'Group' as an example and checks
returned table name is quoted correctly.
@greg0ire greg0ire force-pushed the bugfix/quote_reserved_table_names_in_purger_delete_mode branch from 25a09a8 to 7f309a2 Compare January 4, 2020 16:14
@greg0ire greg0ire changed the title [ORMPurger] Tables with reserved word names are now quoted automatica… Quote table names that also are reserved keywords Jan 4, 2020
@greg0ire greg0ire added this to the 1.4.1 milestone Jan 13, 2020
@greg0ire greg0ire merged commit 640ab13 into doctrine:1.4.x Jan 13, 2020
@stephen-lewis stephen-lewis deleted the bugfix/quote_reserved_table_names_in_purger_delete_mode branch January 15, 2020 19:20
stephen-lewis added a commit to stephen-lewis/data-fixtures that referenced this pull request Jan 17, 2020
Change in 1.4.1 (doctrine#334) caused ORMPurger to generate quotes inside table
names with mocked schemas leaving them invalid (e.g.
test_schema__"group") becuase quoting strategy was ignored. Reverted
changes to ORMPurger::getTableName and added new fucntion to mimic
behaviour of SchemaTool in doctrine/orm when generating delete purge
SQL. Unit tests included to check for correct SQL generation for tables
both with and without schema. Fixes doctrine#338.
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

Successfully merging this pull request may close these issues.

3 participants