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

#2390 - SCRIPT command sometimes exports CREATE VIEW statement before related CREATE TABLE statements #2982

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

warchil
Copy link

@warchil warchil commented Dec 21, 2020

Implementation of SCRIPT command was changed to use new comparator (see ScriptCommand.TableComparator inner class) for ordering of tables/views during the export. Previously the implementation was fully depending on the "database ID". The new comparator is explicitly raising priority of tables and exports them first - before all views.

…ent before related CREATE TABLE statements

- Implementation of SCRIPT command was changed to use new comparator for ordering of tables/views during the export. Previously the implementation was fully depending on the "database ID". The new comparator is explicitly raises priority of tables and exports them first - before all views.
@katzyn
Copy link
Contributor

katzyn commented Dec 21, 2020

Thank you for your attempt to resolve this issue, but I'm not sure about such implementation. It fixes problems in some schemes, but it breaks others. Tables may also depend on views it different ways, for example, through default, on update, and even generated expressions:

CREATE TABLE T(ID BIGINT, V BIGINT);
CREATE VIEW V AS TABLE T;
CREATE TABLE X(ID BIGINT, V BIGINT,  C BIGINT DEFAULT (SELECT COUNT(*) FROM V WHERE V.V = V));

(It's just a reduced example, I saw much more complex meaningful dependencies in some production databases.)

The correct way to resolve the mentioned issue is to ask each table and view about their dependencies and export objects without dependencies first, then export object those depend only on these objects, and so on.

@warchil
Copy link
Author

warchil commented Dec 21, 2020

Thank you very much for quick and very valuable response! You're fully right - I didn't realize that such kind of dependency is also possible.

Do you know if dependencies between tables/views are already calculated by current implementation and are already available somewhere (in the form of some collection(s) for example) so that they can be used in the implementation of improved sorting algorithm for SCRIPT command?

@katzyn
Copy link
Contributor

katzyn commented Dec 21, 2020

H2 has methods to collect dependencies of objects, but I don't remember whether they check dependencies of table columns or not.

There are also invalid views, they need to be exported at the end, because they cannot be analyzed.

@warchil
Copy link
Author

warchil commented Dec 21, 2020

Thank you for describing problematic scenarios - it's obvious that the problem is not trivial. The only fully reliable solution would really be based on full evaluation of dependencies, calculation of dependency graph (in this case it would be probably "forest" graph structure) and then export objects - it looks to me that "reverse" Breadth First Search algorithm could do the job.

In the long run I would try to learn more about this and contribute better solution if possible. Nevertheless from my point of view the current implementation of SCRIPT command which is fully based on ordering by "database ID" is really fragile and might not work even with quite small and trivial database schemas with just a few simple tables and views. So practically the SCRIPT command is not usable for bigger projects and that's a pity because it is really handy feature. I'm using it e.g. for restoring DB back to a defined state before every automated test.

Couldn't we evaluate whether it would make sense to accept my solution or some similarly straightforward solution - just to improve the situation in the short term and make the SCRIPT command more usable? I understand that it is breaking all cases where tables are depending on views - but this is not something what is so frequently used in every project. But for all other much more frequent scenarios (tables with no dependencies on views + views defined on the basis of tables or other views) my solution works whether better or at least in the same way as now because I'm still using "database ID" as a secondary sorting criteria.

@grandinj
Copy link
Contributor

The existing code works well for pretty large projects already, and it already writes views after tables, so I'm not exactly sure why you are seeing this problem.

Do you have any kind of test-case?

@katzyn
Copy link
Contributor

katzyn commented Dec 25, 2020

@grandinj
It is possible to run into this issue after some modifications of the schema, because current behavior depends on initial creation and deletion order of objects.

But changes from this PR will definitely break some valid use cases (and will fix some others), I think we can't replace one bug with another. A better fix is needed.

@warchil
Copy link
Author

warchil commented Jan 5, 2021

First of all, Happy New Year and thank you for your feedback!

@grandinj
Yes, it seems to be exactly like @katzyn wrote. If DB objects like tables are created/deleted multiple times and the problematic view is depending on them then the SCRIPT command might not generate statements in proper sequence. It is not as rare as it could look like. In my scenario I'm using Liquibase tool to maintain DB versioning since the beginning of the project. When I start the application for integration testing I "replay" all Liquibase change sets since the very beginning. Unfortunately (especially during agile development) it happens that objects like tables with the same name might be created, dropped and re-created again during the project life-cycle.

@katzyn
At the moment I can't provide complete fix for all cases as we discussed above. Therefore I ended up using workaround. I'm using SCRIPT command after application startup to store clean starting snapshot of my DB schema. Then I perform following three steps before every test case:

  1. DROP ALL OBJECTS
  2. Execute RUNSCRIPT command to restore initial from the snaphot.
  3. Explicitly execute "ALTER VIEW <VIEW_NAME> RECOMPILE" for every problematic view.

It is not nice but it is sufficient for me now. I hope I can find more time later to help with the real fix.

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.

None yet

3 participants