eulerto / pgquarrel

pgquarrel compares PostgreSQL database schemas (DDL)
BSD 3-Clause "New" or "Revised" License
388 stars 42 forks source link

Detect view definition changes #40

Open rafalcieslak opened 6 years ago

rafalcieslak commented 6 years ago

pgquarrel detects differences in function bodies, but differences in view definitions are ignored. This tiny fix makes it output DROP/CREATE commands for views whose definitions do not match. The definition is compared as text, similarly to how functions are compared.

Running tests on my machine turned out to be quite a challenge, but I got them to work eventually, and it looks like this new test case is processed correctly.

eulerto commented 6 years ago

@rafalcieslak You should use CREATE OR REPLACE VIEW to avoid breaking other objects that depends on that view. Take a look at the function implementation.

Since 9.3 the pg_get_viewdef() changes the way it does line wrapping and indenting. It would emit false positives if you are comparing < 9.3 and >= 9.3. Could you add a comment at the top of the comparison?

rafalcieslak commented 6 years ago

I've tried that initially, but, unfortunately, OR REPLACE on views does not always work. For example, if a column name changes (like in the test case provided), the view cannot be updated with OR REPLACE. You could use ALTER ... RENAME to change the column name, but it's not a universal solution, because e.g. views that are UNIONs cannot be altered this way. Thus I suspect drop&create may be the only strategy that works in all scenarios...

However, choosing this strategy means that in order to correctly deal with dependencies, one would need to drop such dependent objects, drop&create the view, and then recreate the objects. Which would be, obviously, quite a challenge to perform correctly.

So I settled for a solution that works well at least for views that are not used by other objects. Does that make sense?

eulerto commented 6 years ago

However, choosing this strategy means that in order to correctly deal with dependencies, one would need to drop such dependent objects, drop&create the view, and then recreate the objects. Which would be, obviously, quite a challenge to perform correctly.

We currently don't generate a script that cannot be applied. If there is a case, there is one or more bugs around. If we cannot deal with dependencies, let's warn the user.

After sending the last message I realized that we cannot emit CREATE OR REPLACE VIEW before we check some conditions (column order, data types). Also, we don't store view columns yet.

pgquarrel does not understand dependencies yet (I'm planning this feature to the next major version). However, you could check for the object dependencies in the from database and if there is not then we can emit DROP VIEW followed by CREATE VIEW. Otherwise, a warning should be emitted.

rafalcieslak commented 6 years ago

That sounds good to me. I will be looking forward to full dependency support!

In the meantime, I have added some logic that checks whether there are other objects that depend on the view (normal dependencies except for internal dependencies), and in such case a warning is emitted instead of dropping the view.