Skip to content

django: implement DatabaseIntrospection.get_primary_key_column() and add get_relations() stub #320

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Mar 13, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 9 additions & 14 deletions spanner/dbapi/autocommit_off_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -160,27 +160,22 @@ def __update_ddl(self, ddl_statements):
return self.__dbhandle.update_ddl(ddl_statements).result()

def list_tables(self):
# We CANNOT list tables with
# SELECT
# t.table_name
# FROM
# information_schema.tables AS t
# WHERE
# t.table_catalog = '' and t.table_schema = ''
# with a transaction otherwise we get back:
# 400 Unsupported concurrency mode in query using INFORMATION_SCHEMA.
# hence this specialized method.
self.run_prior_DDL_statements()

with self.__dbhandle.snapshot() as snapshot:
res = snapshot.execute_sql("""
return self.run_sql_in_snapshot("""
SELECT
t.table_name
FROM
information_schema.tables AS t
WHERE
t.table_catalog = '' and t.table_schema = ''
""")

def run_sql_in_snapshot(self, sql):
# Some SQL e.g. for INFORMATION_SCHEMA cannot be run in read-write transactions
# hence this method exists to circumvent that limit.
self.run_prior_DDL_statements()

with self.__dbhandle.snapshot() as snapshot:
res = snapshot.execute_sql(sql)
return list(res)

def get_table_column_schema(self, table_name):
Expand Down
13 changes: 3 additions & 10 deletions spanner/dbapi/autocommit_off_cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -357,18 +357,11 @@ def __run_prior_DDL_statements(self):
return self.__connection.run_prior_DDL_statements()

def list_tables(self):
# We CANNOT list tables with
# SELECT
# t.table_name
# FROM
# information_schema.tables AS t
# WHERE
# t.table_catalog = '' and t.table_schema = ''
# with a transaction otherwise we get back:
# 400 Unsupported concurrency mode in query using INFORMATION_SCHEMA.
# hence this specialized method.
return self.__connection.list_tables()

def run_sql_in_snapshot(self, sql):
return self.__connection.run_sql_in_snapshot(sql)

def get_table_column_schema(self, table_name):
return self.__connection.get_table_column_schema(table_name)

Expand Down
25 changes: 10 additions & 15 deletions spanner/dbapi/autocommit_on_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,27 +89,22 @@ def run_prior_DDL_statements(self):
return self.__handle_update_ddl(ddl_statements)

def list_tables(self):
# We CANNOT list tables with
# SELECT
# t.table_name
# FROM
# information_schema.tables AS t
# WHERE
# t.table_catalog = '' and t.table_schema = ''
# with a transaction otherwise we get back:
# 400 Unsupported concurrency mode in query using INFORMATION_SCHEMA.
# hence this specialized method.
self.run_prior_DDL_statements()

with self.__dbhandle.snapshot() as snapshot:
res = snapshot.execute_sql("""
SELECT
return self.run_sql_in_snapshot("""
SELECT
t.table_name
FROM
information_schema.tables AS t
WHERE
t.table_catalog = '' and t.table_schema = ''
""")

def run_sql_in_snapshot(self, sql):
# Some SQL e.g. for INFORMATION_SCHEMA cannot be run in read-write transactions
# hence this method exists to circumvent that limit.
self.run_prior_DDL_statements()

with self.__dbhandle.snapshot() as snapshot:
res = snapshot.execute_sql(sql)
return list(res)

def get_table_column_schema(self, table_name):
Expand Down
13 changes: 3 additions & 10 deletions spanner/dbapi/autocommit_on_cursor.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,18 +269,11 @@ def __run_prior_DDL_statements(self):
return self.__db_handle.run_prior_DDL_statements()

def list_tables(self):
# We CANNOT list tables with
# SELECT
# t.table_name
# FROM
# information_schema.tables AS t
# WHERE
# t.table_catalog = '' and t.table_schema = ''
# with a transaction otherwise we get back:
# 400 Unsupported concurrency mode in query using INFORMATION_SCHEMA.
# hence this specialized method.
return self.__db_handle.list_tables()

def run_sql_in_snapshot(self, sql):
return self.__db_handle.run_sql_in_snapshot(sql)

def get_table_column_schema(self, table_name):
return self.__db_handle.get_table_column_schema(table_name)

Expand Down
3 changes: 0 additions & 3 deletions spanner/django/features.py
Original file line number Diff line number Diff line change
Expand Up @@ -251,9 +251,6 @@ class DatabaseFeatures(BaseDatabaseFeatures):
# DatabaseIntrospection.get_table_description() isn't fully implemented:
# https://github.com/orijtech/django-spanner/issues/248
'introspection.tests.IntrospectionTests.test_get_table_description_col_lengths',
# DatabaseIntrospection.get_primary_key_column() isn't implemented:
# https://github.com/orijtech/django-spanner/issues/312
'introspection.tests.IntrospectionTests.test_get_primary_key_column',
# DatabaseIntrospection.get_relations() isn't implemented:
# https://github.com/orijtech/django-spanner/issues/311
'introspection.tests.IntrospectionTests.test_get_relations',
Expand Down
45 changes: 45 additions & 0 deletions spanner/django/introspection.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,48 @@ def get_table_description(self, cursor, table_name):
)

return descriptions

def get_relations(self, cursor, table_name):
# TODO: PLEASE DO NOT USE THIS METHOD UNTIL
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a slight preference for omitting this implementation for now (and instead return an empty dict) and add this code as part of #313 so the test can be enabled at the same time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The problem though is that digging up that code will require a context switch back and given the amount of time it took for me to dig through those docs I'd rather have it in here but with that warning, and not risk mistakenly going back to investigate again tediously. It'll return {} anyways.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think either of us would forget. Just put the method in a comment on #313. Going to have to edit the comment anyway. Your choice.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the input! Given the amount of time that I spent going back into re-investigating the DATE->TIMESTAMP conversions, I am now wary about my ability to recover code that had been previously posted up, so for that I shall just merge this as is.

# https://github.com/orijtech/django-spanner/issues/313
# is resolved so that foreign keys can be supported, as documented in:
# https://github.com/orijtech/django-spanner/issues/311
"""
Return a dictionary of {field_name: (field_name_other_table, other_table)}
representing all relationships in the table.
"""
results = cursor.run_sql_in_snapshot(
'''
SELECT
tc.COLUMN_NAME as col, ccu.COLUMN_NAME as ref_col, ccu.TABLE_NAME as ref_table
FROM
INFORMATION_SCHEMA.KEY_COLUMN_USAGE AS tc
JOIN
INFORMATION_SCHEMA.REFERENTIAL_CONSTRAINTS as rc
ON
tc.CONSTRAINT_NAME = rc.CONSTRAINT_NAME
JOIN
INFORMATION_SCHEMA.CONSTRAINT_COLUMN_USAGE as ccu
ON
rc.UNIQUE_CONSTRAINT_NAME = ccu.CONSTRAINT_NAME
WHERE
tc.TABLE_NAME="%s"''' % self.connection.ops.quote_name(table_name),
)
return {column: (referred_column, referred_table) for (column, referred_column, referred_table) in results}

def get_primary_key_column(self, cursor, table_name):
results = cursor.run_sql_in_snapshot(
'''
SELECT
ccu.COLUMN_NAME
FROM
INFORMATION_SCHEMA.TABLE_CONSTRAINTS as tc
RIGHT JOIN
INFORMATION_SCHEMA.KEY_COLUMN_USAGE
AS
ccu ON tc.CONSTRAINT_NAME = ccu.CONSTRAINT_NAME
WHERE
tc.TABLE_NAME="%s" AND tc.CONSTRAINT_TYPE='PRIMARY KEY' AND tc.TABLE_SCHEMA=''
''' % self.connection.ops.quote_name(table_name),
)
return results[0][0] if results else None