Remove managerName, use %s, add foldpgID, implement helperFunctions

[afig]

This pull request fixes the inconsistencies we had with case and quote sensitivity with role and schema names.

• The managerNameparameter has been removed from createDBManager
• Wherever a dynamic SQL statement is referencing a role name from a parameter, %s is used instead of the old %I
• A new function has been defined in addHelperFunctions.sql: foldPgID(VARCHAR(65)). This function replicates Postgres's folding behavior. This return value of this function should not be used as an identifier. Instead, it should be used wherever the role or schema name is being used as a string literal. The exact behavior of the function is best seen through the tests that have been added to testAddHelperFunctions.sql. Here is a list of the assertions being made:

  classdb.foldPgID('test') == 'test'
  classdb.foldPgID('Test') == 'test'
  classdb.foldPgID('"test"') == 'test'
  classdb.foldPgID('"Test"') == 'Test'
  classdb.foldPgID('""Test""') == '"Test"'

  In the actual tests, these are conditions in IF blocks and the == are replaced with <>

• Use the new foldPgID function wherever it is needed. Mainly with pg_has_role and when checking for the existence of a role in pg_roles. However there were a couple of other situation where it was needed, such as when INSERTing or DELETEing into/from the Instructor and Student tables, and in the listTables and describe functions in metaFunctions.sql.

I am not certain about foldPgID as the name of the function, but it might suffice for now. One oddity with the function is that the input parameter is VARCHAR(65), but it returns a VARCHAR(63). This is to allow for the maximum length of an identifier (63), and surrounding quotes. The surrounding quotes would be removed before returning the value. There could be a bit more safety with the function so that it always returns a maximum of 63 characters. There is no check that an input that is 64 or 65 characters long actually is quoted. Another issue is that Postgres does not check or even truncate a parameter value that exceeds the size of the VARCHAR(65), nor does it check that the function returns a value this is 63 characters or less.

There should be no issues with merging at this point. Future code may require the use of foldPgID, and should follow the new convention of using %s.

Fixes #85 and #86.

[smurthys]

Nice idea to have a function foldPgID.

I am wondering if we could take advantage of this change to replace the many instances of the query SELECT * FROM pg_catalog.pg_roles WHERE rolname = classdb.foldPgID($1) with a call to classdb.isRoleDefined. Further, could some of the helper functions call foldPgID so the callers don't have to explicitly call foldPgID everywhere?

[afig]

That would be a great idea. I will add those changes onto this branch since similar changes are being made anyways.

It would be possible to have some helper functions use foldPgID. Of course the definition of foldPgIDhas to come first if that is the case.

I forgot to mention this above, but we also need to solve the circular dependency between addHelperFunctions and prepareClassDB. Specifically, functions in prepareClassDB need the functions located in addHelperFunctions, however, addHelperFunctions needs the classdb schema, which is created in prepareClassDB. I do not recall if we decided how to best solve this situation.

[smurthys]

I have thought about many alternatives to the issue of creating the classdb schema. Although not elegant, the quickest fix is to move the code to create classdb schema to addHelperFunctions.sql.

I believe I have a better alternative, but I propose proceeding with the aforementioned quick fix (after analyzing it of course). I will send out a mail with the details of the alternative.

[afig]

Yes there would be an issue with values provided by the Postgres, since it does not quote values that should be case-sensitive. This can be avoided by only using foldPgID with values provided by a user.

[wildtayne]

Thanks for the clarification. I also didn't consider the fact that pgFoldID() returns string data, so it wouldn't work in that context anyway. It's definitely a useful function.