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

Use executeUpdate instead of executeQuery for write operation #880

Closed
wants to merge 20 commits into from

Conversation

goetas
Copy link
Member

@goetas goetas commented Dec 1, 2019

Q A
Type improvement
BC Break no
Fixed issues -

Summary

Use executeUpdate for migration queries as it is a write operation

SenseException and others added 6 commits November 20, 2019 23:08
…branches

Fix of branch name for doctrine website build
* use multiple migrations directories and namespaces

* update more commands

* increase version

* allow custom sorting

* executor

* remove start migration

* value object comparison for versions

* adapted execute command

* do not load from sub namespaces

* format migration state

* start adapting tests

* stricter execution result

* create MigratorConfigurationFactory

* extract factory interface

* fix event dispatcher

* use configuration loaders, drop inheritance

* add config loader tests

* config tests

* executor tests

* command tests

* up to date command

* migrations repo tests

* metadata tests

* alias resolver tests

* factory test

* cs and tests

* interface

* autocommit listener

* migration plan calculator tests

* simplify code for migration plan calculator

* more tests

* rollup test

* restore file builder

* test schema diff

* stub schema provider

* dump schema test

* generate test

* diff command

* dix deps

* execute command

* migrate command

* remove outdated tests

* simplify class

* Update documentation

* more changes

* add event arg test

* static analysis

* moved new-migrations and unavailable migrations to the plan calculator

* green tests

* fix most of the phpstan stuff

* update xsd validator for 3.0

* phpstan

* add constructor provided migrations

* add missing range feature

* fix some failing tests

* missing factory method

* add config loader

* first/last tests

* di test

* move migrator option to command

* migrator tests

* phpstan and tests

* restore down migration behaviour

* stof code review suggestions

* use datetime immutable

* mark value objects as final

* single queries are run in debug mode

* ensure proper typecasting

* skip migration version table in the schema dumps

* allow to filter tables in a dump

* prepare upgrading document

* version it is a string

* fix too large column size

* change output levels

* fixed a lot of stuff from code review

* move generateVersionNumber out of config class

* rename MigratorConfigurationFactory

* rename Factory

* cs

* add version equals method

* rename interfaces

* more readable executed at code

* ensure lowercase column names

* document migrator interface

* code reviews by @greg0ire

* alphabetical sorting of loaders

* change argument order of ConnectionHelperLoader

* rename PhpFileLoader

* remove param default

* mark getDirectoryRelativeToFile as final

* rename UnableToLoadResource

* mark ConfigurationLoader as internal

* split table storage documentation

* MigrationNotAvailable::forVersion

* update composer lock

* handle possible regex errors

* diff command tests fixed after rebase

* upgrade

* run cs on php 7.2

* windows tests

* filesystem sorting is not relevant in the finder

* migration plan execution is immutable

* warn to not use registerMigrationInstance

* regex finder accepts any class naming

* do not allow invalid config keys

* array loader is a hard dependency for the other loaders

* no invalid keys are allowed

* refer to https://github.com/Roave/BackwardCompatibilityCheck instead of having a long list

* classgenerator test

* rename AliasResolver into DefaultAliasResolver

* rename AbstractCommand into DoctrineCommand

* removed cs rules for outdated classes

* renamed ConfigurationHelperInterface into ConfigurationHelper

* renamed ConnectionLoaderInterface into ConnectionLoader

* removed cs rules for outdated classes

* renamed ParameterFormatter into InlineParameterFormatter

* renamed FileBuilderInterface into ConcatenationFileBuilder

* renamed ExecutorInterface into Executor

* ensure there is always some metadata configuration set

* renamed Migrator into DbalMigrator

* add typehint on version class

* fixed typo in MetadataStorageConfiguration class

* re sort imports after class renaming
mikey179/vfsStream is not used anymore in
@goetas goetas added this to the 3.0.0 milestone Dec 1, 2019
@greg0ire
Copy link
Member

greg0ire commented Dec 1, 2019

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

@greg0ire
Copy link
Member

greg0ire commented Dec 1, 2019

Also, since there is no BC-break, this should probably target 2.2.x , can another contributor confirm this?

Drop php 7.1 and allow phpunit 8
@goetas goetas force-pushed the execute-update branch 3 times, most recently from ac84073 to d176b54 Compare December 1, 2019 14:05
@alcaeus
Copy link
Member

alcaeus commented Dec 2, 2019

@greg0ire that is correct - this could easily go into 2.3.0. @goetas do you see any reason why this shouldn't be in 2.3?

@goetas
Copy link
Member Author

goetas commented Dec 3, 2019

I think that this could perfectly go into 2.x too.

Allow dump schema when migrations in other namespaces are present
This avoids having other services hacking into the DI container or config object at runtime (avoiding inconsistent retrieval of services
Make configuration and dependency injection container immutable
@alcaeus
Copy link
Member

alcaeus commented Dec 3, 2019

Sounds good - can you rebase this onto 2.3.x please? Thanks!

@greg0ire greg0ire changed the base branch from master to 2.3.x December 5, 2019 19:43
@greg0ire
Copy link
Member

greg0ire commented Dec 5, 2019

Oooh it's a duplicate in fact, closing see #888

@greg0ire greg0ire closed this Dec 5, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants