rebus-org / Rebus.Oracle

:bus: Oracle transport for Rebus
https://mookid.dk/category/rebus
Other
5 stars 10 forks source link

Better table detection code #10

Closed jods4 closed 5 years ago

jods4 commented 5 years ago

OracleMagic.GetTableNames is used to detect if the storage tables already exist or not.

Current implementation

It's implemented as select table_name from USER_TABLES into a List<string> followed by .Contains with various table names.

I counted at least 5 different calls.

There are some funny variations, such as converting to a HashSet: https://github.com/rebus-org/Rebus.Oracle/blob/16daa0fbd20991233de1f32f531a849db6f471b4/Rebus.Oracle/Oracle/Sagas/OracleSagaSnapshotStorage.cs#L67

But then only probing it once, which is worse than linear search Contains on the list. Also, this instance doesn't seem to care about being case insensitive, although others do.

Here it's converted into a HashSet as well: https://github.com/rebus-org/Rebus.Oracle/blob/16daa0fbd20991233de1f32f531a849db6f471b4/Rebus.Oracle/Oracle/Sagas/OracleSqlSagaStorage.cs#L52

Which is then probed twice, but not by key access! Using key enumeration, which means the Hashset construction is useless.

This is quite an inefficient way to test for existence of a table. Consider that some schemas contain hundreds of tables!

Digression about case insensitivity

Oracle schema is case-sensitive. But if you use unquoted object names, such as up_table, then they are automatically converted to uppercase (so "real" table name is UP_TABLE), which makes it look case-insensitive.

When using quoted names (to use special characters or reserved names), such as "low_table" then the object name is exactly the quoted string (in a case-sensitive way).

(It follows that such an object can not be referenced without quotes, unless the quoted name happens to be all-caps; Conversely you must uppercase any unquoted name: "UP_TABLE" would work but "up_table" does not).

I assume that this library doesn't support quoted names, because it doesn't try to remove quotes when probing user_tables. Looking for "low_table" would fail, as the object name is low_table (without quotes).

Suggested implementation

Replace List<string> GetTableNames() by bool TableExists(string tableName).

Implementation is simply checking if the following query returns a row or none: select 1 from user_tables where table_name = upper(:tableName)

This is assuming that only unquoted names are supported, which as I said are always considered all-caps by Oracle.

If you wanted to support quoted names, then you'd need to check if the name is enclosed in double-quotes; remove them if it is; call ToUpper if it is not. Then just do where table_name = :tableName.

PS

It would be nice to have a copy of all SQL creation scripts in single file somewhere in this repo.

I know you want a "work out-of-the-box" experience, but security best practices is to connect with a user that does not have DDL rights.