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.

Sign up for GitHub

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

Jump to bottom

Fix: Ensuring correct ALTER TABLE statement for creation of an AUTO INCREMENT column as new PRIMARY KEY #3311

Merged
merged 1 commit into from
Dec 7, 2018
Merged

Fix: Ensuring correct ALTER TABLE statement for creation of an AUTO INCREMENT column as new PRIMARY KEY #3311

merged 1 commit into from
Dec 7, 2018

Conversation

arnegroskurth
Copy link
Contributor

Q A
Type bug
BC Break no
Fixed issues #2807

Summary

Ensures that the primary key is created within the same "alter table" statement that an auto-increment column is added to the table as part of the new primary key.

Before the fix for this problem this resulted in a database error:

SQLSTATE[42000]: Syntax error or access violation: 1075 Incorrect table definition; there can be only one auto column and it must be defined as a key

@arnegroskurth
Copy link
Contributor Author

Will probably also resolve issue 3245 of doctrine/doctrine2

@arnegroskurth arnegroskurth changed the title Ensuring correct ALTER TABLE statement for creation of an AUTO INCREMENT column as new PRIMARY KEY Fix: Ensuring correct ALTER TABLE statement for creation of an AUTO INCREMENT column as new PRIMARY KEY Oct 4, 2018
@arnegroskurth arnegroskurth mentioned this pull request Oct 4, 2018
Open
morozov
morozov reviewed Oct 5, 2018
View reviewed changes
Copy link
Member

@morozov morozov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@arnegroskurth, thank you for the patch. Please see a few comments.

tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL2807Test.php Outdated Show resolved Hide resolved
tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL2807Test.php Outdated Show resolved Hide resolved
tests/Doctrine/Tests/DBAL/Functional/Ticket/DBAL2807Test.php Outdated Show resolved Hide resolved
@arnegroskurth
Copy link
Contributor Author

@morozov The test now asserts the correct resulting schema and is no longer restricted to mysql

@morozov
Copy link
Member

morozov commented Oct 14, 2018

@arnegroskurth please look at the failing tests. Before getting the fix merged, the tests should be fixed for all supported platforms or marked as skipped. In the latter case, please also file a new issue or issues describing the reason of the failure.

@arnegroskurth
Copy link
Contributor Author

arnegroskurth commented Oct 15, 2018 •
edited
Loading

@morozov The test is now restricted to mysql again. I think this made sense in the first place as I fixed the problem only within Doctrine\DBAL\Platforms\MysqlPlatform anyway.

@arnegroskurth arnegroskurth mentioned this pull request Oct 15, 2018
Open
@arnegroskurth
Copy link
Contributor Author

@morozov Is there anything else you need to merge this?

@morozov
Copy link
Member

morozov commented Nov 5, 2018

@arnegroskurth please resolve #3311 (comment) and move the new test under the DBAL\Functional\Platform namespace. Naming tests after issue numbers is not informative. Also, please fix the build (should be fixed by a rebase).

@arnegroskurth
Copy link
Contributor Author

@morozov The test is now renamed, moved and your code style has been applied. As already discussed the skip for non-mysql platforms IS necessary as the fix only covers the mysql platform. The follow-up ticket for other platforms already exists.

@morozov
Copy link
Member

morozov commented Nov 5, 2018

@arnegroskurth I was referring to the return statement. It's not necessary since markTestSkipped() throws an exception.

@arnegroskurth
Copy link
Contributor Author

@morozov The tests now failing are not related to my changes

@morozov
Copy link
Member

morozov commented Dec 5, 2018 •
edited
Loading

@arnegroskurth I've restarted the errored jobs. Such hiccups sometimes happen on Travis, and we cannot do anything about them.

@arnegroskurth
Copy link
Contributor Author

@morozov I had to re-add the return-statement to meet the codestyle requirements

@morozov morozov self-assigned this Dec 6, 2018
@morozov
Copy link
Member

morozov commented Dec 6, 2018

@arnegroskurth as long as you're okay with the changes I pushed, please squash all commits, and I'll merge it.

@arnegroskurth
Copy link
Contributor Author

@morozov Done

@morozov morozov merged commit f0e93fb into doctrine:master Dec 7, 2018
@morozov morozov added this to the 2.10.0 milestone Dec 7, 2018
@morozov
Copy link
Member

morozov commented Dec 7, 2018

🚢

@djaiss djaiss mentioned this pull request Nov 9, 2019
Closed
rgrellmann added a commit to Rossmann-IT/dbal that referenced this pull request Apr 22, 2020
@rgrellmann
Merge tag 'v2.10.0' into oracle122
84cc190 
# Release v2.10.0

This is a minor release of Doctrine DBAL that aggregates over 70 fixes and improvements developed by 25 contributors over the last year.

This release focuses on internal code quality improvement and deprecating the functionality identified for removal in the next major release.

## Backwards Compatibility Breaks

This release introduces a minor BC break. Default column values are no longer handled as SQL expressions. They are converted to SQL literals (e.g. escaped). The previous behavior was not portable and was never by design.

Clients must now specify default values in their initial form, not in the form of an SQL literal (e.g. escaped).

**Before:**

```php
$column->setDefault('Foo\\\\Bar\\\\Baz');
```

**After:**

```php
$column->setDefault('Foo\\Bar\\Baz');
```

## Deprecations

- The usage of the `getDriver()`, `getDatabasePlatform()` and `getSchemaManager()` methods of the `ConnectionEventArgs` class has been deprecated.
- The usage of the `getDatabasePlatform()` method of the `SchemaColumnDefinitionEventArgs` class has been deprecated.
- The usage of the `getHost()`, `getPort()`, `getUsername()` and `getPassword()` methods of the `Connection` class has been deprecated.
- Passing multiple SQL statements as an array to `SchemaAlterTableAddColumnEventArgs::addSql()` and the same method in other `SchemaEventArgs`-based classes is deprecated.
- Calling `AbstractSchemaManager::tablesExist()` with a string argument is deprecated.
- Calling `OracleSchemaManager::createDatabase()` without an argument or by passing `NULL` is deprecated.
- Unused schema manager methods are deprecated.
   -  `AbstractSchemaManager::_getPortableFunctionsList()`,
   - `AbstractSchemaManager::_getPortableFunctionDefinition()`,
   - `OracleSchemaManager::_getPortableFunctionDefinition()`,
   - `SqliteSchemaManager::_getPortableTableIndexDefinition()`.
- The usage of `NULL` to indicate empty `$username` or `$password` when calling `Doctrine\DBAL\Driver::connect()` is deprecated.
- Method `Doctrine\DBAL\Platforms::_getAlterTableIndexForeignKeySQL()` has been deprecated as no longer used.
- Property `Doctrine\DBAL\Driver\OCI8\OCI8Statement::$_PARAM` has been deprecated as not used.
- Method `Doctrine\DBAL\Driver::getName()` is deprecated.
- The usage of user-provided `PDO` instance is deprecated.
- `Type::*` constants are deprecated.
- The `Doctrine\DBAL\Driver\SQLSrv\SQLSrvStatement::LAST_INSERT_ID_SQL` constant has been deprecated.
- The constants in `Doctrine\DBAL\SQLParserUtils` have been deprecated.
- The `Doctrine\DBAL\Logging\LoggerChain::addLogger` method has been deprecated.

Please see the details in the [UPGRADE.md](UPGRADE.md) documentation.

## New Features and Improvements

- [3674: Add missing MySQL 8.0 reserved keywords](doctrine#3674) thanks to @loilo
- [3512: Support for comments on table in all databases](doctrine#3512) thanks to @moufmouf
- [3418: Add column charset for MySql](doctrine#3418) thanks to @altertable

**MySQL-related changes:**

 - [3668: Quote collation on MySQL](doctrine#3668) thanks to @staudenmeir
 - [3374: Clean up `MySqlPlatform::getListTableIndexesSQL()` fields](doctrine#3374) thanks to @BenMorel
 - [3311: Ensuring correct `ALTER TABLE` statement for creation of an `AUTO INCREMENT` column as new `PRIMARY KEY`](doctrine#3311) thanks to @arnegroskurth

**Driver level changes:**

- [3677: Relax statement type declaration](doctrine#3677) thanks to @greg0ire
- [3521: Maintain platform parameter in connection params](doctrine#3521) thanks to @jwage and @Perf
- [3588: Add commit result bool](doctrine#3588) thanks to @otazniksk

**Schema management:**

- [2960: Handle default values as values, not SQL expressions](doctrine#2960) thanks to @morozov

**Types improvements:**

 - [3356: Extract constants for built-in types from Type to Types](doctrine#3356) thanks to @Majkl578
 - [3354: Extract type factory and registry from Type into TypeRegistry](doctrine#3354) thanks to @Majkl578

**Compatibility with Symfony 5:**

 - [3706: add missing exit codes to ensure Symfony 5 compatibility](doctrine#3706) thanks to @dmaicher
 - [3563: Allow Symfony 5](doctrine#3563) thanks to @nicolas-grekas

**Query Builder:**

 - [3696: Add support for `DISTINCT` clause](doctrine#3696) thanks to @bingo-soft

**Logging:**

 - [3572: Make LoggerChain use constructor to add loggers instead of adder method](doctrine#3572) thanks to @ostrolucky

**Code quality improvements:**

 - [3667: Phpstan fix backport](doctrine#3667) thanks to @morozov
 - [3663: Updated PHPStan to v0.11.15](doctrine#3663) thanks to @morozov
 - [3604: Updated Jetbrains PhpStorm stubs to 2019.1](doctrine#3604) thanks to @morozov
 - [3549: Removed the assertion which doesn't work with a user-provided PDO connection](doctrine#3549) thanks to @morozov
 - [3489: Update doctrine coding standard from 5.0 to 6.0](doctrine#3489) thanks to @amaabdou
 - [3481: Updated PHPStan to v0.11.3](doctrine#3481) thanks to @morozov
 - [3443: PHPStan Level 7](doctrine#3443) thanks to @morozov
 - [3442: PHPStan Level 6](doctrine#3442) thanks to @morozov
 - [3436: PHPStan Level 5](doctrine#3436) thanks to @morozov
 - [3435: PHPStan Level 4](doctrine#3435) thanks to @morozov
 - [3432: Updated PHPStan to v0.11](doctrine#3432) thanks to @morozov

**Test suite improvements:**

 - [3705: Don't skip a test for sqlite](doctrine#3705) thanks to @Federkun
 - [3689: Updated PHPUnit to 8.4.1](doctrine#3689) thanks to @morozov
 - [3664: Refactor usage of MockBuilder's deprecated setMethods()](doctrine#3664) thanks to @baspeeters
 - [3643: Bumped PHPUnit requrement to ^8.3.3, removed dependency on symfony/phpunit-bridge](doctrine#3643) thanks to @morozov
 - [3609: Reworked the mocks generated by Prophecy using PHPUnit](doctrine#3609) thanks to @morozov
 - [3608: Added a unit test for Doctrine\DBAL\Logging\LoggerChain](doctrine#3608) thanks to @morozov
 - [3600: Updated PHPUnit to 8.2.1](doctrine#3600) thanks to @morozov
 - [3575: Enforced parameter and return value types in test classes](doctrine#3575) thanks to @morozov
 - [3566: Upgraded to PHPUnit 8.1.6 and reworked the remaining driver exception mock](doctrine#3566) thanks to @morozov
 - [3555: Removed the rest of mock classes](doctrine#3555) thanks to @morozov
 - [3546: Reworked driver exception tests](doctrine#3546) thanks to @morozov
 - [3530: Improve ExceptionTest::testConnectionExceptionSqLite](doctrine#3530) thanks to @jwage
 - [3474: Remove more hard-coded mock classes](doctrine#3474) thanks to @morozov
 - [3470: Replaced MockPlatform with the ones generated by PHPUnit](doctrine#3470) thanks to @morozov
 - [3468: Marked some test classes abstract](doctrine#3468) thanks to @morozov
 - [3446: Upgraded PHPUnit to 8.0](doctrine#3446) thanks to @morozov

**Documentation improvements:**

 - [3616: Fix typo in docblock](doctrine#3616) thanks to @rdarcy1
 - [3559: add .github/FUNDING.yml](doctrine#3559) thanks to @jwage
 - [3556: Removed 2.8 from README](doctrine#3556) thanks to @morozov
 - [3514: Expand list of keywords in composer.json](doctrine#3514) thanks to @Majkl578
 - [3504: fix doctrine#3479 (typos in example-code in QueryBuilder)](doctrine#3504) thanks to @DavidBruchmann
 - [3503: Fix the branch alias for master](doctrine#3503) thanks to @stof
 - [3463: fixed a typo in PoolingShardConnection phpdoc](doctrine#3463) thanks to @adapik
 - [3408: Removed unused build files](doctrine#3408) thanks to @morozov
 - [3404: Link to Slack instead of Gitter](doctrine#3404) thanks to @greg0ire
 - [3376: Bump version to 2.10.0-DEV](doctrine#3376) thanks to @morozov

**CI improvements:**

 - [3688: Temporarily disable the usage of PHPUnit 8.4 due to a regression](doctrine#3688) thanks to @morozov
 - [3654: fix Appveyor builds](doctrine#3654) thanks to @mmucklo
 - [3644: Using command line options to configure MySQL 8 instead of mounting a config file](doctrine#3644) thanks to @morozov
 - [3509: Enabled the build against IBM DB2 on PHP 7.3 on Travis CI](doctrine#3509) thanks to @morozov
 - [3528: Reworked SQL Server installer on Travis CI](doctrine#3528) thanks to @morozov
 - [3484: Use latest stable versions of sqlsrv and pdo&doctrine#95;sqlsrv on CI](doctrine#3484) thanks to @morozov
 - [3475: Make PHP 7.3 the primary PHP version on Travis CI](doctrine#3475) thanks to @morozov
 - [3473: Avoid database connection from PHPUnit data providers](doctrine#3473) thanks to @morozov
 - [3469: Replaced Xdebug with PCOV for code coverage](doctrine#3469) thanks to @morozov
 - [3405: Travis CI: PHP 7.3 tests on MySQL 8.0](doctrine#3405) thanks to @BenMorel
 - [3394: Grouped PHPStan and PHP&doctrine#95;CodeSniffer for parallel execution](doctrine#3394) thanks to @morozov
 - [3388: Require PHP 7.2, drop <7.2 in Composer & on CI](doctrine#3388) thanks to @morozov
 - [3122: Introduced a smoke testing phase on Travis to run SQLite tests and CS checks first](doctrine#3122) thanks to @morozov

**Deprecations:**

 - [3708: New deprecations for 2.10](doctrine#3708) thanks to @morozov and @jwage
 - [3598: Deprecated some unused code bits](doctrine#3598) thanks to @morozov
 - [3558: Deprecated Driver::getName()](doctrine#3558) thanks to @morozov
 - [3554: The usage of user-provided PDO instance is deprecated](doctrine#3554) thanks to @morozov
 - [3542: Deprecate SQLSrvStatement::LAST&doctrine#95;INSERT&doctrine#95;ID&doctrine#95;SQL constant.](doctrine#3542) thanks to @jwage
 - [3541: Deprecate the public constants in SQLParserUtils](doctrine#3541) thanks to @jwage

# gpg: Signature made Sun Nov  3 17:56:11 2019
# gpg:                using RSA key 374EADAF543AE995
# gpg: Can't check signature: public key not found

# Conflicts:
#	.travis.yml
#	README.md
#	lib/Doctrine/DBAL/Platforms/SQLAnywherePlatform.php
#	lib/Doctrine/DBAL/Schema/MySqlSchemaManager.php
#	lib/Doctrine/DBAL/Schema/Table.php
#	lib/Doctrine/DBAL/Version.php
#	phpcs.xml.dist
#	tests/Doctrine/Tests/DBAL/Functional/ConnectionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/ExceptionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/MasterSlaveConnectionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/Platform/DefaultExpressionTest.php
#	tests/Doctrine/Tests/DBAL/Functional/ResultCacheTest.php
#	tests/Doctrine/Tests/DBAL/Platforms/SQLAnywherePlatformTest.php
#	tests/Doctrine/Tests/DBAL/Schema/ComparatorTest.php
#	tests/travis/mysql.docker.travis.xml
#	tests/travis/mysqli.docker.travis.xml
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 10, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@morozov morozov morozov approved these changes

Assignees

@morozov morozov

Labels
Bug Indexes MySQL Schema Comparison
Projects
None yet
Milestone
2.10.0
Development

Successfully merging this pull request may close these issues.
2 participants
@arnegroskurth @morozov