DASSL / ClassDB

An open-source system to let students experiment with relational data
https://dassl.github.io/ClassDB/
Other
7 stars 2 forks source link

Guard uses of ALTER SYSTEM: changed in pg9.4 (N) #228

Closed smurthys closed 6 years ago

smurthys commented 6 years ago

This issue is based on a finding by @KevinKelly25.

ALTER SYSTEM was introduced in Postgres 9.4 and thus its execution should be guarded by the server version number. It appears that prior to Postgres 9.4, it is necessary to change postgresql.conf to get the same effect as using ALTER SYSTEM.

ALTER SYSTEM is presently used in enableConnectionLoggingReco.psql and disbleConnectionLoggingReco.psql.

smurthys commented 6 years ago

Because enableConnectionLoggingReco.psql and disbleConnectionLoggingReco.psql do all their work using ALTER SYSTEM, it seems like a good idea to check Postgres server version at the beginning of the scripts.

SELECT current_setting('server_version') or SHOW server_version returns server version number.

BTW, we need to find which version each configuration parameter (for example, server_version) itself was introduced in. Both current_setting and SHOW cause a run-time error if a non-existent parameter name is queried.

An alternative is to use the pg_settings view as follows to avoid a run-time error. This query returns no rows if a configuration parameter is not defined. Perhaps this query/functionality could be a wrapped in a helper function.

SELECT setting FROM pg_catalog.pg_settings 
WHERE name = 'server_version';
KevinKelly25 commented 6 years ago

The server_version parameter is present in 9.3 and from the docs it seems to have been introduced in 7.4 along with many of the current parameters.

All 4 parameters at the beginning of enableConnectionLoggingReco.psql are also present in 9.3

A more in depth look comparing the parameters in 9.3 vs 10.4 I found there is 231 parameters in 9.3 and 271 in 10.4. From what I can tell the changes/additions do not effect this issue. It seems like version shouldn't be an issue for referring to a parameter, at least in terms of syntax.

While not necessarily related to this issue I compiled all the differences in this gist.

smurthys commented 6 years ago

Thanks @KevinKelly25 for the gist. I assume you generated the lists by comparing the names pg_settings view in 9.3 and 10.4. Am I correct?

I wonder if a list of parameters available in each version is readily available somewhere without having to install each version.

KevinKelly25 commented 6 years ago

Yeah, you are correct, I compared the results from SELECT name FROM pg_catalog.pg_settings; I looked around for a list of parameters for each version but couldn't find anything. There was a decent list for each version maintained until 8.0 found here. However, If you go any further they break it up into sections which is still helpful but not exactly the greatest for comparisons between versions

afig commented 6 years ago

Although all instances ALTER SYSTEM can likely be replaced with SET to easily allow for pre 9.4 compatibility, doing so is not ideal since we wish to implement the use of newer and more supported functionality. Given that 9.3 will no longer be supported after September of 2018 (one month from now), it also may not be worth the effort to support 9.3.

The decision made in a meeting was to add guards that raise an exception in enableConnectionLoggingReco.psql and disableConnectionLoggingReco.psql if the server version is < 9.4, while ensuring that the psql variable ON_ERROR_STOP is enabled. These exceptions should raise a message indicating that the current version is not supported by ClassDB, and possibly state or point to documentation that further describes the actions needed to attempt use with older server versions. If time allows, additional scripts that allow for pre-9.4 support for scripted connection logging can be created.