PostgreSQL best practices compliance

[frcroth]

The people who made PostgreSQL have created this document: https://wiki.postgresql.org/wiki/Don%27t_Do_This#Text_storage

Some things I noticed

• Char should never be used -> We use it a lot
• Varchar(n) should probably not be used -> We use it

Opinions on this @fm3 ?

[fm3]

Yeah, I saw the document too yesterady, but didn’t read through it yet. We should go through it at some point and consider what we should change (and probably not just text).

I think it doesn’t have super high priority, though. After all, we’ve been sailing fairly smoothly so far with our db.

[fm3]

Looks like we are guilty of

• NOT IN (in wk app code)
• Uppercase Chars in table and column names (in our schema.sql)
• CHAR(n) (in our schema.sql)
• VARCHAR(n) (in our schema.sql) where we could use text. Consider limiting size of user-supplied strings already in Controllers/Actions instead.

I always thought that we should probably eventually move our timestamptz fields to timestamp (without timezone), as they are all utc anyway, but apparently that is not true

[MichaelBuessemeyer]

While we are at it, we could unfiy the style in which enum case strings are written. We have Capitalized, Small Case and all caps 🥴

[fm3]

I think we need to do this in multiple steps. I would suggest the following order, possibly in separate PRs:

• get rid of NOT IN in the wk app code (unless with explicit list). Rewrite with NOT EXISTS
  • FolderDAO.moveSubtree
  • UserDAO.buildSelectionPredicates
• replace data types CHAR/VARCHAR with text
• Change table names
  • What to do with the views that currently end with underscore?
• Change column names
  • Probably split into multiple PRs, this can break a lot of stuff if done without care
  • What to do with columns currently starting with underscores? Leave as is? Or rename, e.g. _task to task_id, _dataStore to data_store_name?
    • If so, also change the fields in representing case classes, double-checking that they are not auto-serialized in public APIs.
• Change enum values to unify style
  • I’d only change them in the DB if we can change them everywhere. No conversion on DB I/O.
  • But some of them are used in public APIs?

[frcroth]

TBH I do not see the issue with enum formatting, I think the different styles also represent different things.

CREATE TYPE webknossos.JOB_STATE AS ENUM ('PENDING', 'STARTED', 'SUCCESS', 'FAILURE', 'CANCELLED');
The enum represents a State, which are usually written in uppercase

CREATE TYPE webknossos.ANNOTATION_TYPE AS ENUM ('Task', 'Explorational', 'TracingBase', 'Orphan');
This represents a type, there is no reason for it to be fully uppercase

CREATE TYPE webknossos.LENGTH_UNIT AS ENUM ('yoctometer', 'zeptometer', 'attometer', 'femtometer', 'picometer', 'nanometer', 'micrometer', 'millimeter', 'centimeter', 'decimeter', 'meter', 'hectometer', 'kilometer', 'megameter', 'gigameter', 'terameter', 'petameter', 'exameter', 'zettameter', 'yottameter', 'angstrom', 'inch', 'foot', 'yard', 'mile', 'parsec');
This is directly a unit, these units are officialy written lower case, so it makes sense here.

There are some violations (ANNOTATION_STATE does not have uppercase elements), but I can see a logic behind it.

[frcroth]

Removed occurences of CHAR and VARCHAR as well as NOT IN in #8372