prepareClassDB: Latest version in comments-consistency-fixes-M1 results in errors when run (W)

[afig]

Attempting to run the prepareClassDB script results in the transaction being aborted due to a few syntax errors in the file, and an incompatibility with an older version. The specific problems are listed below:

• CREATE OR REPLACE does not work if the functions have different parameter names. This means that in its current state the script cannot be run over an older version with different parameter names. This can be fixed by adding a DROP FUNCTION IF EXISTS prior to the affected functions.

• Line 102 uses %1 to refer to a parameter in the createUser function. This resulted in an error when running the script. It can be fixed by using $1 instead. Here is a copy of the line: format('CREATE USER %I ENCRYPTED PASSWORD %L', %1, COALESCE($2, $1));

• Lines 294, 347, and 380: It seems like there is a missing AND between the two boolean expressions. Here is an example: WHERE pg_catalog.pg_has_role($1, oid, 'member') rolname != $1

• The ON CONFLICT clause resulted in an error when attempting to execute the createStudent and createInstructor functions. The error is copied below. It seems like there was an issue with username being both the name of a parameter and of a column in each table. A quick fix was to change the name of the parameter, but we may want to choose a different solution.

  ERROR:  column reference "username" is ambiguous
  LINE 2:           ON CONFLICT (username) DO UPDATE SET studentname =...
                            ^
  DETAIL:  It could refer to either a PL/pgSQL variable or a table column.
  QUERY:  INSERT INTO classdb.Student VALUES($1, $2, $3)
        ON CONFLICT (username) DO UPDATE SET studentname = $2
  CONTEXT:  PL/pgSQL function createstudent(

[afig]

The last three points have been resolved in commits c1a4d7c13c24a60325edcead4c413cc8987ff403, 2c365c0db51315a1641981283b777465abe0ed13, and e448264cb1c567c2a6b358143f9f6be36dbf1212 .

The first point is being ignored for now. This is because any change in the name or type of a parameter would require using a similar facility. Thus, every function would likely be preceded by multiple DROP FUNCTION IF EXISTS statements, depending on how many revisions it has had. A better solution to this problem might be to have a cleanup script. For now, manually running a DROP FUNCTION statement should be sufficient. This situation should generally only occur during development.

[smurthys]

I am a bit concerned about the need to always quote role names and it is incompatible with the use of the name PUBLIC. It was an error on my part to use %I: I didn't intend to. :(. mea maxima culpa

DROP FUNCTION could be required when functions are patched, and patches should be expected (for example, on the DASSL VM). I would consider prefixing each CREATE FUNCTION with a DROP FUNCTION instead of using CREATE OR REPLACE FUNCTION. I also advise studying the behavior of DROP FUNCTION with and without parameter names included.

[afig]

Using %s instead of %I does not automatically double quote the role names when using the format function. It might be a better choice to use this in the pg_temp.createGroupRoles function in prepareClassServer. By using this, we will not have to quote role names, and we can use the capitalization we were using prior.

Improper formatting and SQL Injection should not be an issue as the function is created and dropped within the same script (and transaction).

[afig]

Fixed by d974bb7869e0dcb7bcbcbc5a0f381d0ad6ad1bb7, merged into master in 916e05feac2512acccf1395ff769455f4be62fe8