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 uses of pg_event_trigger_ddl_commands(): changed in pg9.5 (N) #234

Closed afig closed 6 years ago

afig commented 6 years ago

The pg_event_trigger_ddl_commands() function was added in version 9.5.

This function is used in ClassDB.logDDLActivity in addDDLActivtiyLoggingReco.sql to obtain information about the DDL activity performed.

Unfortunately, the release notes seem to suggest that this information was not available at all prior to 9.5.

Due to this issue, ClassDB can be fully installed on versions prior to 9.5, but an ERROR is raised whenever a DDL activity is performed by a ClassDB user:

classdb=> CREATE TABLE testtable(Col1 VARCHAR);
ERROR:  function pg_event_trigger_ddl_commands() does not exist
LINE 2:                               FROM pg_event_trigger_ddl_comm...
                                           ^
HINT:  No function matches the given name and argument types. You might need to add explicit type casts.
QUERY:  SELECT object_identity --Get the statement target object
                              FROM pg_event_trigger_ddl_commands() --can only be called on non-DROP ops
         WHERE object_identity IS NOT NULL
         ORDER BY object_identity
CONTEXT:  PL/pgSQL function logddlactivity() line 10 at SQL statement
afig commented 6 years ago

In a meeting, it was determined that the likely best course of action is to set objId to an empty string (not NULL) instead of selecting from pg_eventTrigger_ddlcommands() on L58-L62 of addDDLActivityLoggingReco.sql for server versions < 9.5. Doing so will allow L78 of the same file to evaluate as true and correctly insert a row into ClassDB.DDLActivity (albeit with an empty DDLObject name).

KevinKelly25 commented 6 years ago

After reviewing the code and trying to create an alternative for server versions pre-9.5 I have found that changing objId to an empty string is not a valid alternative because of a check in the DDLActivity table.

Following is causing the failure and is from the DDLActivity table in addUserMgmtCore.sql Lns 58-67:

DDLObject VARCHAR NOT NULL --name of the object of the DDL operation
            CHECK(TRIM(DDLObject) <> '')

More research is needed but so far the only alternatives I have seen so far is:

1) Remove the check from DDLObject in the DDLActivity Table

2) Use something like N/A as the objId. Since it is not a valid identifier name for a DDL object it should never show up in the table except for instances where version is an issue.

smurthys commented 6 years ago

@KevinKelly25 Please proceed with N/A as the value for objId, because we should not loosen the schema just to support older pg versions.

I don't know if N/A is an appropriate value, but I can't think of a better value right now. Just - comes to my mind, but N/A seems more direct and appropriate than -.

Suggestions?