-
-
Notifications
You must be signed in to change notification settings - Fork 286
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
Incremental Changes Support #245
Comments
Added "Diffing" section. Probably that's all for now; I'm ready for feedback. |
Thank you @XedinUnknown for the detailed description, I would like to confirm following questions from my perspedtive.
I agree with you about the diffing part, which is quite similar to what I would imagine for incremental changes. If this is possible, the migrations generator should run the incremental changes automatically; hence, the DBML creation should be handled automatically by the generator. Assuming we could generate state I imagine we could upgrade the generator to read the existing migration files to create state
I think this is a bit too much for the migrations generator, which should focus on
This is a huge challenge for implementation. We must never allow the generator to produce any
Yes, I agree on the problem.
Yes, but this requires too much rename/alter information to be provided to the generator for significant changes. If such information is already recorded, why not have the user create migration files manually instead?
We should not modify the comments of tables/fields/indexes, as the owner may utilize them for their own explanation or description purposes. My thoughtThe proposed implementation brings an interesting way to utilize DBML for diff checking. However, it still has unresolved challenges that I believe are not easily solvable at this moment:
|
I'm not sure what you mean by running the incremental changes automatically. But I disagree that the DBML creation should be handled automatically by the generator. Perhaps, the rest of my response will provide more details as to why.
Currenly, state
Therefore, the only that remains to be serialized is state
The workflow for adding incremental changes could therefore be:
In other words, it is the responsibility of the consumer to provide state
I disagree with the above statement. It's perfectly valid for a table added in v1 of a software to be removed in v2, with no renaming or anything like that.
The generator does not modify any database; it only writes changes to migrations. It is up to the consumer (and always has been) to review those changes before committing them and applying them to their database.
It requires some information, yes. But if migrations from state
I agree, we should not. But we should allow the consumer to specify that they want the generator to use metadata as source of information about object identity, in which case they can specify that information themselves. We can provide a guide as to what works best. So to address the points in your summary:
|
Problem
This library can be used to create migrations that set up a DB schema from scratch.
It cannot be used to create migrations that bring the DB from one non-empty state to another. I.e. it cannot create migrations that would add incremental changes based on another existing schema.
Solution
Add support for generation of migrations, which would configure the schema from an arbitrary state, not only from an empty state.
States
A migration can be seen as a diff: it encodes all of the changes that have to be made to state
A
in order to bring it to stateB
. At present, stateA
= 0, and there is no way to reliably use any other state as a base for changes.Of course, even in the current implementation, migrations can be used on top of a non-empty DB. However, the existing state of the DB may not be compatible with these migrations: creating a table with a field of a specified name and type would fail if the DB already contains a table with that name which has that field. In this example scenario, the expected solution would be: to alter the existing table, either by adding a new field (if it does't exist yet), or by changing an existing field to match the specified configuration. The way forward therefore seems to be: allow specifying an arbitrary state
A
, to which migrations could be applied in order to bring it into stateB
.State Representation
At present, state
B
is represented by a live DB: the generated migrations execute queries that bring a non-conflicting live databaseA
into the same state as an existing live databaseB
. This is non-ideal, because it requires an accessible database, but still works: most Laravel projects would work with such a database, and its initialization is trivial with supporting tools.The same cannot be said about state
A
: the requirement to have 2 live databases in different states is not part of any usual workflow, and not something that can easily be achieved. Therefore, it could make sense to save a serialized representation of stateA
, in order to compare it to stateB
.Note
When that is achieved, it appears relatively simple to serialize state
B
as well, which could make this package more versatile and efficient; however, that is outside of the scope of this proposal.DBML
The DataBase Markup Language is the only widely used DSL for documenting cross-platform DB schemas, making it the de facto interop standard for interchanging such information. The
butschster/dbml-parser
package is a not tremendously outdated parser for it, making reading the schema from this fromat easy. Theegyjs/dbml-to-laravel
already uses DBML to do codegen, and according to its docs it does a pretty good job at that. Therefore, I suggest using DBML for schema serialization.Note
egyjs/dbml-to-laravel
claims to generate enums that are then used in other generated code. This integrated approach is very beneficial, and I would love to see your tool move in the same direction. However, that is out of scope for this feature request.Diffing
Assuming that the schemas of both states
A
andB
are available via a PHP API, here's what diffing could look like.B
that is not inA
.A
B
?B
that is not inA
.A
:B
?B
different toA
?B
that is not inA
.Renaming
It isn't clear what to do about tables, fields, and indexes (collectively, "objects") that have been renamed (constraints contain no data, so they don't appear to be a concern here). If
my_table
has been renamed tomy_cool_table
, it will dropmy_table
and addmy_cool_table
; same would go for other named things. This is very sub-optimal, and sort of defeats the point of having incremental migrations: to preserve existing data while changing the underlying schema. Renamed things should lead to an alteration in the migration, rather than being deleted and re-added with a different name.At the same time, it is unclear how renaming should be implemented: it doesn't seem possible to compare 2 schemas and tell with certainty whether an object in
B
is a renamed object inA
or not. They may be very similar in structure but have different purposes; they may also have a slightly different name but a significantly different structure.This seems to dictate that additional information is necessary in order for an intended migration to be generated. Where this information comes from - is a topic of its own, but it seems like it may be any combination of the below.
config
dir of Laravel projects.This has the problem that it shouldn't change frequently between codegen runs.
This is good because every run that generates incremental migrations could specify its own renames: they may differ from one run to another.
B
as it is inA
, or a different one.This is good because database objects themselves may specify the identifier representing their significance once, it would be attached directly to the object (and would survive a re-import to/from MySQL dump etc), and any run would yield expected results without specifying additional information.
References
The text was updated successfully, but these errors were encountered: