Guard uses of GRANT: changed in pg9.5 (N)

[KevinKelly25]

Using postgreSQL version 9.3, initializeDBCore.sql has a syntax error on line 95. As shown here:

psql:core/initializeDBCore.sql:95: ERROR:  syntax error at or near "CURRENT_USER"
LINE 1: GRANT ClassDB TO CURRENT_USER;

It looks like in version 9.5 the syntax for using GRANT changed as seen here by comparing versions.

[afig]

The change in the syntax only affects direct use of CURRENT_USER or SESSION_USER. These role_specifications can still be used if the statement is dynamically generated. Our biggest use of CURRENT_USER is as a default parameter. This use is unaffected by the change in syntax.

The change in syntax also affects various other commands, including REVOKE, ALTER ROLE, DROP OWNED BY, and REASSIGN OWNED BY

There is only one statement in ClassDB's source that is affected (the same one pointed out above). There are 6 more affected uses of CURRENT_USER in the test scripts (5 in testDisallowSchemaDrop.sql and 1 in testRoleBaseMgmt.sql.

A potential workaround is converting the affected uses of CURRENT_USER into dynamic statements, which will result in Postgres calling CURRENT_USER before generating the statement, avoiding syntax issues. Since they involve the use of the EXECUTE statement and FORMAT() function, they can only be called in a plpgsql block. For example, the issue above can be mediated by replacing the affected code on L95 with the following:

--Grant ClassDB to the current user
-- allows altering privileges of objects, even after being owned by ClassDB
DO
$$
BEGIN
   EXECUTE FORMAT('GRANT ClassDB TO %s', CURRENT_USER);
END
$$;

Of course, this comes with all the drawbacks of using a dynamically generated statement (no caching, increased difficulty in debugging, and no syntax highlighting in a text editor).

Once that one use of CURRENT_USER has been updated, ClassDB can be installed without issues caused by the change in syntax. Modifying the 5 instances in the test scripts allows them to run (they do not pass due to an unrelated issue). I do not foresee any other issues cause by this change.

[afig]

During a meeting, it was determined that the ideal solution is to implement dynamic code like that in the previous comment, but guard the use of it to only when it is necessary (for server versions < 9.5).

This will involve adding guarded use of dynamic code near L95 of initializeDBCore.sql, near L107 L140 L170 L201 L241 of testDisallowSchemaDrop.sql, and L137 of testRoleBaseMgmt.sql.

[smurthys]

Just noting that a helper function ClassDB.grantRole already exists to safely grant a role to another role (and to raise a clearer customer exception). This function uses dynamic SQL.

EDIT: In the context of this issue, the helper function is useful only in test scripts.

[smurthys]

I created a separate issue #269 to capture a similar change in DROP OWNED.