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

Detect view definition changes #40

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rafalcieslak
Copy link
Contributor

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
Copy link
Owner

eulerto commented Mar 14, 2018

@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
Copy link
Contributor Author

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
Copy link
Owner

eulerto commented Mar 15, 2018

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
Copy link
Contributor Author

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.

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.

2 participants