- 
                Notifications
    
You must be signed in to change notification settings  - Fork 31
 
          chore: unit test coverage and additional refactoring for spanner_dbapi
          #532
        
          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
Changes from 39 commits
92cc0b3
              c70ee50
              da4eb61
              d27ff71
              eaa024f
              3cde688
              75c6f1c
              4383d1c
              f05307d
              a5d6d30
              2693582
              dd38239
              03b440e
              4640c46
              023ea90
              9abee91
              7dc3875
              02b3bd4
              5e0ee36
              628f04a
              58c0bb6
              301ca84
              2e5ba9b
              b4fde4b
              d585579
              7d8b016
              a73e1d7
              9337fd8
              b535bf1
              81db6f7
              d484496
              4c0b83c
              6580e6a
              4b7f7d8
              a6fcbdf
              b38982a
              594c9bc
              da15d94
              2a78830
              41abaeb
              File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | 
|---|---|---|
| @@ -0,0 +1,159 @@ | ||
| # Copyright 2020 Google LLC | ||
| # | ||
| # Use of this source code is governed by a BSD-style | ||
| # license that can be found in the LICENSE file or at | ||
| # https://developers.google.com/open-source/licenses/bsd | ||
| 
     | 
||
| from google.cloud.spanner_dbapi.parse_utils import get_param_types | ||
| from google.cloud.spanner_dbapi.parse_utils import parse_insert | ||
| from google.cloud.spanner_dbapi.parse_utils import sql_pyformat_args_to_spanner | ||
| from google.cloud.spanner_v1 import param_types | ||
| 
     | 
||
| 
     | 
||
| SQL_LIST_TABLES = """ | ||
| SELECT | ||
| t.table_name | ||
| FROM | ||
| information_schema.tables AS t | ||
| WHERE | ||
| t.table_catalog = '' and t.table_schema = '' | ||
| """ | ||
| 
     | 
||
| SQL_GET_TABLE_COLUMN_SCHEMA = """SELECT | ||
| COLUMN_NAME, IS_NULLABLE, SPANNER_TYPE | ||
| FROM | ||
| INFORMATION_SCHEMA.COLUMNS | ||
| WHERE | ||
| TABLE_SCHEMA = '' | ||
| AND | ||
| TABLE_NAME = @table_name | ||
| """ | ||
| 
     | 
||
| # This table maps spanner_types to Spanner's data type sizes as per | ||
| # https://cloud.google.com/spanner/docs/data-types#allowable-types | ||
| # It is used to map `display_size` to a known type for Cursor.description | ||
| # after a row fetch. | ||
| # Since ResultMetadata | ||
| # https://cloud.google.com/spanner/docs/reference/rest/v1/ResultSetMetadata | ||
| # does not send back the actual size, we have to lookup the respective size. | ||
| # Some fields' sizes are dependent upon the dynamic data hence aren't sent back | ||
| # by Cloud Spanner. | ||
| code_to_display_size = { | ||
| param_types.BOOL.code: 1, | ||
| param_types.DATE.code: 4, | ||
| param_types.FLOAT64.code: 8, | ||
| param_types.INT64.code: 8, | ||
| param_types.TIMESTAMP.code: 12, | ||
| } | ||
| 
     | 
||
| 
     | 
||
| def _execute_insert_heterogenous(transaction, sql_params_list): | ||
| for sql, params in sql_params_list: | ||
| sql, params = sql_pyformat_args_to_spanner(sql, params) | ||
| param_types = get_param_types(params) | ||
| res = transaction.execute_sql( | ||
| sql, params=params, param_types=param_types | ||
| ) | ||
| # TODO: File a bug with Cloud Spanner and the Python client maintainers | ||
| # about a lost commit when res isn't read from. | ||
| 
         
      Comment on lines
    
      +57
     to 
      +58
    
   
  There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know your change just moved this comment, but I wonder what the story is here. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It would be a good question for the original code developer who unfortunately is no longer a member of our team. I'm not even sure what's the purpose of   | 
||
| _ = list(res) | ||
| 
     | 
||
| 
     | 
||
| def _execute_insert_homogenous(transaction, parts): | ||
| # Perform an insert in one shot. | ||
| table = parts.get("table") | ||
| columns = parts.get("columns") | ||
| values = parts.get("values") | ||
| return transaction.insert(table, columns, values) | ||
| 
     | 
||
| 
     | 
||
| def handle_insert(connection, sql, params): | ||
| parts = parse_insert(sql, params) | ||
| 
     | 
||
| # The split between the two styles exists because: | ||
| # in the common case of multiple values being passed | ||
| # with simple pyformat arguments, | ||
| # SQL: INSERT INTO T (f1, f2) VALUES (%s, %s, %s) | ||
| # Params: [(1, 2, 3, 4, 5, 6, 7, 8, 9, 10,)] | ||
| # we can take advantage of a single RPC with: | ||
| # transaction.insert(table, columns, values) | ||
| # instead of invoking: | ||
| # with transaction: | ||
| # for sql, params in sql_params_list: | ||
| # transaction.execute_sql(sql, params, param_types) | ||
| # which invokes more RPCs and is more costly. | ||
| 
     | 
||
| if parts.get("homogenous"): | ||
| # The common case of multiple values being passed in | ||
| # non-complex pyformat args and need to be uploaded in one RPC. | ||
| return connection.database.run_in_transaction( | ||
| _execute_insert_homogenous, parts | ||
| ) | ||
| else: | ||
| # All the other cases that are esoteric and need | ||
| # transaction.execute_sql | ||
| sql_params_list = parts.get("sql_params_list") | ||
| return connection.database.run_in_transaction( | ||
| _execute_insert_heterogenous, sql_params_list | ||
| ) | ||
| 
     | 
||
| 
     | 
||
| class ColumnInfo: | ||
| """Row column description object.""" | ||
| 
     | 
||
| def __init__( | ||
| self, | ||
| name, | ||
| type_code, | ||
| display_size=None, | ||
| internal_size=None, | ||
| precision=None, | ||
| scale=None, | ||
| null_ok=False, | ||
| ): | ||
| self.name = name | ||
| self.type_code = type_code | ||
| self.display_size = display_size | ||
| self.internal_size = internal_size | ||
| self.precision = precision | ||
| self.scale = scale | ||
| self.null_ok = null_ok | ||
| 
     | 
||
| self.fields = ( | ||
| self.name, | ||
| self.type_code, | ||
| self.display_size, | ||
| self.internal_size, | ||
| self.precision, | ||
| self.scale, | ||
| self.null_ok, | ||
| ) | ||
| 
     | 
||
| def __repr__(self): | ||
| return self.__str__() | ||
| 
     | 
||
| def __getitem__(self, index): | ||
| return self.fields[index] | ||
| 
     | 
||
| def __str__(self): | ||
| str_repr = ", ".join( | ||
| filter( | ||
| lambda part: part is not None, | ||
| [ | ||
| "name='%s'" % self.name, | ||
| "type_code=%d" % self.type_code, | ||
| "display_size=%d" % self.display_size | ||
| if self.display_size | ||
| else None, | ||
| "internal_size=%d" % self.internal_size | ||
| if self.internal_size | ||
| else None, | ||
| "precision='%s'" % self.precision | ||
| if self.precision | ||
| else None, | ||
| "scale='%s'" % self.scale if self.scale else None, | ||
| "null_ok='%s'" % self.null_ok if self.null_ok else None, | ||
| ], | ||
| ) | ||
| ) | ||
| return "ColumnInfo(%s)" % str_repr | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW our current isort config will mangle this formatting. Did you do this by hand?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I did. It would work either way, and was more of the matter of a consistent visual appearance.