search

DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
8 stars 2 forks source link

Guard CREATE INDEX IF NOT EXISTS for Postgres 9.4 or earlier #258

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

This commit fixes #230 by using CREATE INDEX IF NOT EXISTS only if the server version is 9.5 or later.

KevinKelly25 commented 6 years ago

This looks really good. However, it does not run correctly on version 9.4. I get the following error:

psql:core/addRoleBaseMgmtCore.sql:100: ERROR:  syntax error at or near "NOT"
LINE 19:       CREATE UNIQUE INDEX IF NOT EXISTS idx_Unique_FoldedRol...

I believe this error is being cause by the fact that Postgres runs a syntax check before it runs the DO. So while it never actually reaches this part of the IF statement it will still get an error. I ensured this was the issue by commenting out the CREATE UNIQUE INDEX IF NOT EXISTS in the if statement and ensuring the other part of the IF statement was being run. Since the index was being created it means that it never actually runs the other part of the IF statement but still raises an error.

We might have to make a separate file for running specific versions to avoid issues like this or run dynamic sql statments.

Edit: Added more Information to error

smurthys commented 6 years ago

@KevinKelly25 Thank you for the review. I am working on a maintainable alternative and push a change.

All: My apologies for not thinking on my own the issue @KevinKelly25 points out. Also, I think this sequence points out the benefits of teamwork particularly when supporting multiple server versions.

smurthys commented 6 years ago

I just pushed a change that dynamically executes CREATE INDEX IF NOT EXISTS so the script compiles with pre-pg9.5. I discarded the alternative of using just the pre-pg9.5 code for all versions to make sure the customization is easily found later.

smurthys commented 6 years ago

I pushed a change to addUserMgmtCore.sql to address #259. Fortunately, the fix turned out to remove the use of IF NOT EXISTS.

I will continue to rely on others to test this on a pre9.6 pg server, because I presently do not have the time to set up multiple pg servers. Thank you.

KevinKelly25 commented 6 years ago

This looks and works really well on 9.4 and 10.4. There is only one small issue that I corrected to test. There is one more instance of ADD COLUMN IF NOT EXISTS on ln 226 in addUserMgmtCore.sql.

Otherwise it works great and all tests pass related to this functionality (some others fail in 9.4 due to other version issues outside of this PR).

smurthys commented 6 years ago

Thanks @KevinKelly25 for catching the omission. I just pushed a change addressing it.

smurthys commented 6 years ago

Thank you for the reviews and for testing on multiple versions.