Guard IF NOT EXISTS in CREATE INDEX for pre-pg9.5

[smurthys]

The changes in this PR guard the use of IF NOT EXISTS in CREATE INDEX.

The changes work in pg9.6, but need to be tested in pg9.3 or 9.4 as well as in 10.x.

Fixes #230

BTW, these changes give a glimpse of the code structure we might need to make the product compatible with Postgre versions prior to 9.6

[wildtayne]

I haven't had to closely examine the code yet, however the version of ClassDB this branch provides installs, and the tests pass, on 10.3.

[KevinKelly25]

10.4 works but 9.3 is still getting

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

I was doing some testing and I think it might be that even though 9.3 never could get there it still does a syntax check on the whole code block before running through it. I tested this out by making a small block of code that contained syntax that 9.3 doesn't use. I put that block of code into an IF statement that would be impossible to get to. Even though it was not possible to get to the illegal syntax it still would get an error every time I ran that test.

I am still testing to see if there is a way to get around this.

EDIT - added a better explaination

[afig]

Nice catch and analysis @KevinKelly25. Looks like there is a workaround. We can tell Postgres that the "invalid" statement is a dynamic command, which means that it will not attempt to cache it, avoiding the syntax error.

L79-L83 of addRoleBaseMgmtCore.sql would look like:

ELSE
   --works on pg9.5 or later
   EXECUTE 'CREATE UNIQUE INDEX IF NOT EXISTS idx_Unique_FoldedRoleName'
            ' ON ClassDB.RoleBase(ClassDB.foldPgID(RoleName))';
END IF;

[smurthys]

Thanks @KevinKelly25 for the report. I was quite expecting this outcome. 😢

The dynamic-sql alternative @afig suggests is an alternative, but we should in general prefer static code over dynamic code, because dynamic code is prone to errors and hard to debug. They do have the advantage that the alternative "code" can stay inline and then later be easily replaced with static code.

Another alternative may be to place any code that works only in a later version inside a temp function, because I believe a function body is late bound. Just so we can verify, please run the following code on pg9.3. Whether we use that approach or not, I am curious to know the outcome.

However, this issue brings in to focus our priority for M3, and also calls for us to make a a long-term strategy on supporting pg versions prior to 9.5. I propose we have that discussion in #227 so our thinking is recorded in one place and remains public.

CREATE OR REPLACE FUNCTION pg_temp.createIndexUniqueFoldedRoleName_afterPg94() 
RETURNS VOID AS
$$
   CREATE UNIQUE INDEX IF NOT EXISTS idx_Unique_FoldedRoleName
   ON ClassDB.RoleBase(ClassDB.foldPgID(RoleName));
$$ LANGUAGE sql;

DO
$$
BEGIN
   IF ClassDB.isServerVersionAfter('9.4') THEN
      PERFORM pg_temp.createIndexUniqueFoldedRoleName_afterPg94();
   ELSE
      --pg9.4
      --works on any pg version, but the post pg9.4 version is much simpler
      IF NOT EXISTS (SELECT indexname FROM pg_catalog.pg_indexes
                     WHERE schemaname = 'classdb'
                          AND tablename = 'rolebase'
                          AND indexname = 'idx_unique_foldedrolename'
                    )
      THEN
         CREATE UNIQUE INDEX idx_Unique_FoldedRoleName
         ON ClassDB.RoleBase(ClassDB.foldPgID(RoleName));
      END IF;
   END IF;
END
$$;

[KevinKelly25]

I ran the previous temp function @smurthys brought up on 9.3. It is still giving the same error upon trying to create the function.

[smurthys]

Thanks @KevinKelly25 for the confirmation.

This confirmation leaves us with two options as far as this issue is concerned:

• Implement just the pre9.5 solution because it works in all version
• Implement different versions for pre9.5 and 9.5+ with the 9.5+ version being a dynamic query

Either approach is acceptable. However, my concern is this issue is not representative of the requirements/complexity/effort needed to support compatibility with previous pg versions. I invite a general discussion at #227.

[smurthys]

Given the decision in #227 to address Postgres compatibility in M4, should we abandon this PR?

It will be clean to address all compatibility issues in M4, and there isn't much effort lost if we abandon now. (I will just stash the work for later use.)

[wildtayne]

I think it makes sense to abandon this PR until M4, since we need to re-implement the changes using one of the options outlined by @smurthys. I agree we don't lose much in terms of code already produced.

[afig]

Closing this PR without merging makes sense. We can reevaluate the solution to compatability problems later, during M4.

[smurthys]

Closing this PR following the decision in #227 to delay compatibility issues to Milestone 4.