This repository has been archived by the owner on Mar 28, 2019. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 15
Replace DROP INDEX IF EXISTS by a DO block #487
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/me wondering: Since we are in a
DO
block, couldn't we catch the creation exception instead (less verbose) ?(It might fill the server logs with meaningless errors though)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean exactly, by now if the index is not exists it will be created, and if it exists nothing will happen. I fixed an error in rodo@1477c25 suppose it's related on what you mean.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, what I mean is that there are two ways to write the same behaviour.
For example, in python:
is equivalent (and less performant btw) to:
So for the index creation I guess it's the same.
Either :
DO $$ BEGIN IF NOT EXISTS ( ... ) THEN CREATE INDEX END IF; END$$;
or
Indeed, it would have prevented the typo you add in the commit you mentionned :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes we can do that, but the log will be more verbose as at each time you'll execute the file on your DB you'll have an error, I think we have a different goal. As a DBA I always want to reduce log amount, I prefer script that run without raising error, even if I have less information, here you want to Raise an error if you try to create an already existing index, the opposite side of view.
We have to define the goal before choosing a solution.
Be careful of the line number 4 that set min_loglevel to ERROR you won't see the NOTICE
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that what I thought in my first comment :)
This works for me ! I just wanted to challenge your choice :)
Thanks for the details !