Add `tables` property to ConnectionWrapper subclasses

[Ryan J Ollos]

While working on #11512, it occurred to me that we might be able to organize the code a bit if the tables query was made to a property of the ConnectionWrapper subclasses.

Would it make sense to also have an interface class for the ConnectionWrapper subclasses (IDatabaseConnection?). It might help anyone that it trying to implement a new database type.

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t11580.

Previously it was possible to specify the path for a MySQLdb connection when calling destroy_db. The path is used as the dbname, which maps to the schema in the SQL statement: tags/trac-1.0.1/trac/test.py@:375,378#L367​ I couldn't see any reason to keep this behavior since the schema from the Postgres connection is used, and the behavior should be the same for all database backends.

Let me if you have any concerns about this.

[Jun Omae]

It would be nice to add feature that retrieve table names from database.

But I like to add as a method rather than tables() decorated @property because I get a feeling that the attribute is an instance variable or a cached property. In addition, most attributes of ConnectionWrapper are methods.

Also, it would be useful for plugins to add get_columns(table) for retrieve column names from table.

I think we cannot add an interface IDatabaseConnection. Currently, all database connection classes aren't Component subclass. However, an interface class in Trac requires Component subclass to implement.

But I think we could an abstract class instead the interface.

[Ryan J Ollos]

Thanks for the feedback. I will prepare some new changes with get_tables and get_columns, following your suggestions.

[Ryan J Ollos]

Latest changes can be found in log:rjollos.git:t11580.2.

I was considering that we might want to return more info than just the column name. For instance, we could return the type and whether it is a primary key. Another possibility though would be to add a get_column_info or similar in the future, if needed.

There are two things I'm unsure of:

• Whether there is a need to quote table names (as in #11512). The tests failed for postgres and mysql when I wrapped tablename with db.quote.
• Whether care needs to be taken for string to unicode conversion, like that shown in: tags/trac-1.0.1/trac/db/api.py@:334-335#L331​.

I should probably add some additional tests in order to determine whether those things are needed.

[Jun Omae]

Latest changes can be found in log:rjollos.git:t11580.2.

In get_column_names() of sqlite_backend.py, I think we should use a quoted identifier.

-        rows = cursor.execute("PRAGMA table_info('%s')" % tablename)
+        rows = cursor.execute("PRAGMA table_info(%s)" % self.quote(tablename))

sqlite> CREATE TABLE `Ho)ur` (`Id` INTEGER PRIMARY KEY);
sqlite> PRAGMA table_info(`Ho)ur`);
0|Id|INTEGER|0||1
sqlite> CREATE TABLE `Ho')u``r` (`Id` INTEGER PRIMARY KEY);
sqlite> PRAGMA table_info(`Ho')u``r`);
0|Id|INTEGER|0||1

In __init__() of mysql_backend.py, I don't think we need use of basename() because a leading slash is stripped at start of __init__(), tags/trac-1.0.1/trac/db/mysql_backend.py@:246-247#L239​.

-        self.schema = os.path.basename(path)
+        self.schema = path

I was considering that we might want to return more info than just the column name. For instance, we could return the type and whether it is a primary key.

Sounds good.

[Ryan J Ollos]

Replying to jomae:

I was considering that we might want to return more info than just the column name. For instance, we could return the type and whether it is a primary key.

Sounds good.

Another idea would be to have get_column_names and get_table_names return only the names since often only the names are needed, but also add a method get_schema that returns the table and column names, as well as the primary keys and column types.

[Ryan J Ollos]

Replying to jomae:

In get_column_names() of sqlite_backend.py, I think we should use a quoted identifier.

I see now. We need to quote the parameter for get_column_names in sqlite_backend.py because string interpolation is performed. However, for the same methods in mysql_backend.py and postgres_backend.py the schema name and table name are passed as parameters so the quoting is performed by the execute function.

Committed to 1.0-stable in [12709:12710] and merged to trunk in [12711:12712].

In the future I'll look into adding a get_schema method.

[Ryan J Ollos]

While working on #11581, I noticed that we should probably use the name table for the get_column_names second positional parameter rather than tablename, for consistency with drop_table, get_last_id and update_sequence.

[Ryan J Ollos]

Renamed variable in [12756], merged in [12757].

[Ryan J Ollos]

#12105 suggests adding get_table_names to the DatabaseManager class.