Need schema support for PostgreSQL

[MB34]

For PostgreSQL databases, you also need to add schema capability. The tables may not always be installed in the default schema.

i.e. mine are in a schema called auth, so this code: (if key = 1)

$bucket = $this->db->selectRow(
	'SELECT tokens, replenished_at FROM ' . $this->dbTablePrefix . 'users_throttling WHERE bucket = ?',
	[ $key ]
);

which produces this SQL:

SELECT tokens, replenished_at FROM users_throttling WHERE bucket = 1

would become this:

$bucket = $this->db->selectRow(
	'SELECT tokens, replenished_at FROM ' . $this->dbTableSchema . '.' . $this->dbTablePrefix . 'users_throttling WHERE bucket = ?',
	[ $key ]
);

which would produce this SQL:

SELECT tokens, replenished_at FROM auth.users_throttling WHERE bucket = 1

[ocram]

Thanks!

We certainly had PostgreSQL’s schemas in mind.

What we wanted is a database-agnostic and driver-agnostic library. So this doesn’t necessarily need the vocabulary that PostgreSQL uses, as long as it supports its features.

Our idea was that you could simply add the schema as a prefix to the dbTablePrefix instead of having a separate dbTableSchema. That means your dbTablePrefix would be auth.users_ and your table name would be throttling. Or your prefix would be auth. and your table name users_throttling.

If there are any serious downsides to this, we’d certainly consider adding a separate variable for this, though. Can you comment on whether there are any problems with our solution in practice?

We thought that tables with a common prefix (and belonging to a common component) would probably share the same schema as well. Is that wrong?

[MB34]

While I can do this, the issue with PostgreSQL is that if you enclose the tablenames in quotes it fails:

SELECT * FROM "table1"

works, but

SELECT * FROM "schema.table1"

doesn't work.

However, commenting out this line, in several places, makes it work:

// escape the table name
$tableName = $this->quoteIdentifier($tableName);

[MB34]

I also don't like having to pass the tablePrefix in the Auth constructor. Should be a property of the Datasource.

[ocram]

Good points, thank you!

It seems we should indeed make this change and introduce a separate schema name.

[MB34]

I did it but it wasn't pretty, still had to pass it in the constructor. I used a ternary to determine if it had one and modified the SQL kind of like this:

$this->db->insert(
    (isset($this->dbTableSchema) ? $this->dbTableSchema . '.' : '') . $this->dbTablePrefix . 'users',
    [
        'username' => $username,
        /* other fields */
    ]
);

But I overwrote all my changes to test out the other stuff... ;-(

[ocram]

Are you suggesting that we should add schema support to this database library here or to our authentication library?

For this database library here, our review shows that only the following three methods are affected:

PdoDatabase#insert
PdoDatabase#update
PdoDatabase#delete

Is that correct?

If so, it seems this would simply be a matter of introducing a schemaName variable (and also a tableNamePrefix variable), prepending those two variables to the tableName variable in the three methods mentioned above, and quoting or escaping all three components separately.

By the way, the reason why our authentication library handles the prefix for the table names itself is that this database library here could support a shared prefix but the authentication library can also be used with plain PDO instances, which don’t support such prefixes.

Further, you may have a shared prefix for the tables used by your authentication component, but a different prefix for other tables used by your application. If we implement the prefix here in this database library, it must be shared by all tables that your application uses.

[MB34]

Are you suggesting that we should add schema support to this database library here or to our authentication library?

No, it should be a property of the DataSource. I understand having a dbTablePrefix being passed to the Auth constructor to allow different prefixes for the Auth library. You can have both. May need to pass a schema, too.

The schema would also have to be added to the SQL in the Auth, Administration, and UserManager classes.
Auth lines:
117
159
356
623
820
834
922
1135
1161
1247
1387
1694
1871

Administration lines:
280
306
460
476
552

UserManager lines:
143
261
406

[ocram]

Thank you!

Support for qualified table names, and thus for non-default schemas in PostgreSQL, has now been added in cf4cd6c and 7a03da2. Specifically, this change affects PdoDatabase#insert, PdoDatabase#update and PdoDatabase#delete.

Adding support for a default schema is something that we may consider in the future, but I’m not sure about that yet. It has to work really well and must be convenient and unambiguous. One thing to consider is that this library allows for the shorthands Database#insert, Database#update and Database#delete but also accepts explicit statements written out in full.

Support for the database schema has also been added to our authentication library now, namely in the form of another argument to the constructor (Sorry!) which should ensure full PostgreSQL support, also with non-default schemas.