chore: Add support for named schema #858
Conversation
product-auto-label
bot
added
the
api: spanner
django_spanner/introspection.py
Outdated
| @@ -38,7 +38,7 @@ class DatabaseIntrospection(BaseDatabaseIntrospection): | |||
| FROM | |||
| information_schema.tables AS t | |||
| WHERE | |||
| t.table_catalog = '' and t.table_schema = '' | |||
| t.table_catalog = '' and t.table_schema = '{schema_name}' | |||
There was a problem hiding this comment.
Is the if USE_EMULATOR statement still needed?
There was a problem hiding this comment.
Removed as the mentioned bug is closed and support has been added to emulator
django_spanner/introspection.py
Outdated
| results = cursor.run_sql_in_snapshot(self.LIST_TABLE_SQL) | ||
| schema_name = self._get_schema_name(cursor) | ||
| results = cursor.run_sql_in_snapshot( | ||
| self.LIST_TABLE_SQL.format(schema_name=schema_name) |
There was a problem hiding this comment.
I know it's not introduced by this PR, but if I understand it correctly, all of these methods generate a SQL statement with a literal in the string, instead of using a query parameter. That means that they are all potentially susceptible to SQL injection. Can we fix that for the queries that we are working on in this PR, and then do a follow-up PR for all other queries? (This should have a high priority)
django_spanner/introspection.py
Outdated
| table=quoted_table_name | ||
| ) | ||
| WHERE TABLE_NAME=@table AND TABLE_SCHEMA=@schema_name""", | ||
| params={"table": quoted_table_name, "schema_name": schema_name}, |
There was a problem hiding this comment.
We should not use a quoted table name when it is used as a query parameter. Also; as there is no test failing due to this, I think it means that this method is also not covered by any tests. Would you mind adding a test for this method as well?
django_spanner/introspection.py
Outdated
| table=quoted_table_name | ||
| ) | ||
| TABLE_NAME=@table AND TABLE_SCHEMA=@schema_name""", | ||
| params={"table": quoted_table_name, "schema_name": schema_name}, |
There was a problem hiding this comment.
Here also: The table name should not be quoted when used as a parameter. Can we have a test for this method as well?
django_spanner/introspection.py
Outdated
| table=quoted_table_name | ||
| ) | ||
| """, | ||
| params={"table": quoted_table_name, "schema_name": schema_name}, |
There was a problem hiding this comment.
Same regarding quoted table name
django_spanner/introspection.py
Outdated
| tc.TABLE_NAME=@table AND tc.TABLE_SCHEMA=@schema_name | ||
| """, | ||
| params={ | ||
| "table": self.connection.ops.quote_name(table_name), |
There was a problem hiding this comment.
here also: table name should not be quoted, and the lack of a test failure to me seems like a lack of test coverage
Co-authored-by: Knut Olav Løite <[email protected]>
No description provided.