Skip to content

Commit ebecc7d

Browse files
tseavertseaver
authored
Drop 'Database.read' and 'Database.execute_sql' convenience methods. (#3787)
Because the context managers they use returned the session to the database's pool, application code could not safely iterate over the result sets returned by the methods. Update docs for 'Snapshot.read' and 'Snapshot.execute_sql' to emphasize iteration of their results sets before the session is returned to the database pool (i.e., within the 'with' block which constructs the snapshot). Closes #3769.
1 parent 64d1728 commit ebecc7d

File tree

4 files changed

+42
-138
lines changed

4 files changed

+42
-138
lines changed

‎docs/spanner/snapshot-usage.rst‎

Lines changed: 32 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -45,12 +45,22 @@ fails if the result set is too large,
4545

4646
.. code:: python
4747
48-
result = snapshot.read(
49-
table='table-name', columns=['first_name', 'last_name', 'age'],
50-
key_set=['[email protected]', '[email protected]'])
48+
with database.snapshot() as snapshot:
49+
result = snapshot.read(
50+
table='table-name', columns=['first_name', 'last_name', 'age'],
51+
key_set=['[email protected]', '[email protected]'])
5152
52-
for row in result.rows:
53-
print(row)
53+
for row in result.rows:
54+
print(row)
55+
56+
.. note::
57+
58+
The result set returned by
59+
:meth:`~google.cloud.spanner.snapshot.Snapshot.execute_sql` *must not* be
60+
iterated after the snapshot's session has been returned to the database's
61+
session pool. Therefore, unless your application creates sessions
62+
manually, perform all iteration within the context of the
63+
``with database.snapshot()`` block.
5464

5565
.. note::
5666

@@ -68,14 +78,24 @@ fails if the result set is too large,
6878

6979
.. code:: python
7080
71-
QUERY = (
72-
'SELECT e.first_name, e.last_name, p.telephone '
73-
'FROM employees as e, phones as p '
74-
'WHERE p.employee_id == e.employee_id')
75-
result = snapshot.execute_sql(QUERY)
81+
with database.snapshot() as snapshot:
82+
QUERY = (
83+
'SELECT e.first_name, e.last_name, p.telephone '
84+
'FROM employees as e, phones as p '
85+
'WHERE p.employee_id == e.employee_id')
86+
result = snapshot.execute_sql(QUERY)
87+
88+
for row in result.rows:
89+
print(row)
90+
91+
.. note::
7692

77-
for row in result.rows:
78-
print(row)
93+
The result set returned by
94+
:meth:`~google.cloud.spanner.snapshot.Snapshot.execute_sql` *must not* be
95+
iterated after the snapshot's session has been returned to the database's
96+
session pool. Therefore, unless your application creates sessions
97+
manually, perform all iteration within the context of the
98+
``with database.snapshot()`` block.
7999

80100

81101
Next Step

‎spanner/google/cloud/spanner/database.py‎

Lines changed: 0 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -313,68 +313,6 @@ def session(self):
313313
"""
314314
return Session(self)
315315

316-
def read(self, table, columns, keyset, index='', limit=0,
317-
resume_token=b''):
318-
"""Perform a ``StreamingRead`` API request for rows in a table.
319-
320-
:type table: str
321-
:param table: name of the table from which to fetch data
322-
323-
:type columns: list of str
324-
:param columns: names of columns to be retrieved
325-
326-
:type keyset: :class:`~google.cloud.spanner.keyset.KeySet`
327-
:param keyset: keys / ranges identifying rows to be retrieved
328-
329-
:type index: str
330-
:param index: (Optional) name of index to use, rather than the
331-
table's primary key
332-
333-
:type limit: int
334-
:param limit: (Optional) maxiumn number of rows to return
335-
336-
:type resume_token: bytes
337-
:param resume_token: token for resuming previously-interrupted read
338-
339-
:rtype: :class:`~google.cloud.spanner.streamed.StreamedResultSet`
340-
:returns: a result set instance which can be used to consume rows.
341-
"""
342-
with SessionCheckout(self._pool) as session:
343-
return session.read(
344-
table, columns, keyset, index, limit, resume_token)
345-
346-
def execute_sql(self, sql, params=None, param_types=None, query_mode=None,
347-
resume_token=b''):
348-
"""Perform an ``ExecuteStreamingSql`` API request.
349-
350-
:type sql: str
351-
:param sql: SQL query statement
352-
353-
:type params: dict, {str -> column value}
354-
:param params: values for parameter replacement. Keys must match
355-
the names used in ``sql``.
356-
357-
:type param_types:
358-
dict, {str -> :class:`google.spanner.v1.type_pb2.TypeCode`}
359-
:param param_types: (Optional) explicit types for one or more param
360-
values; overrides default type detection on the
361-
back-end.
362-
363-
:type query_mode:
364-
:class:`google.spanner.v1.spanner_pb2.ExecuteSqlRequest.QueryMode`
365-
:param query_mode: Mode governing return of results / query plan. See
366-
https://cloud.google.com/spanner/reference/rpc/google.spanner.v1#google.spanner.v1.ExecuteSqlRequest.QueryMode1
367-
368-
:type resume_token: bytes
369-
:param resume_token: token for resuming previously-interrupted query
370-
371-
:rtype: :class:`~google.cloud.spanner.streamed.StreamedResultSet`
372-
:returns: a result set instance which can be used to consume rows.
373-
"""
374-
with SessionCheckout(self._pool) as session:
375-
return session.execute_sql(
376-
sql, params, param_types, query_mode, resume_token)
377-
378316
def run_in_transaction(self, func, *args, **kw):
379317
"""Perform a unit of work in a transaction, retrying on abort.
380318

‎spanner/tests/system/test_system.py‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ def test_update_database_ddl(self):
297297

298298
self.assertEqual(len(temp_db.ddl_statements), len(DDL_STATEMENTS))
299299

300-
def test_db_batch_insert_then_db_snapshot_read_and_db_read(self):
300+
def test_db_batch_insert_then_db_snapshot_read(self):
301301
retry = RetryInstanceState(_has_all_ddl)
302302
retry(self._db.reload)()
303303

@@ -310,10 +310,7 @@ def test_db_batch_insert_then_db_snapshot_read_and_db_read(self):
310310

311311
self._check_row_data(from_snap)
312312

313-
from_db = list(self._db.read(self.TABLE, self.COLUMNS, self.ALL))
314-
self._check_row_data(from_db)
315-
316-
def test_db_run_in_transaction_then_db_execute_sql(self):
313+
def test_db_run_in_transaction_then_snapshot_execute_sql(self):
317314
retry = RetryInstanceState(_has_all_ddl)
318315
retry(self._db.reload)()
319316

@@ -329,7 +326,8 @@ def _unit_of_work(transaction, test):
329326

330327
self._db.run_in_transaction(_unit_of_work, test=self)
331328

332-
rows = list(self._db.execute_sql(self.SQL))
329+
with self._db.snapshot() as after:
330+
rows = list(after.execute_sql(self.SQL))
333331
self._check_row_data(rows)
334332

335333
def test_db_run_in_transaction_twice(self):
@@ -346,7 +344,8 @@ def _unit_of_work(transaction, test):
346344
self._db.run_in_transaction(_unit_of_work, test=self)
347345
self._db.run_in_transaction(_unit_of_work, test=self)
348346

349-
rows = list(self._db.execute_sql(self.SQL))
347+
with self._db.snapshot() as after:
348+
rows = list(after.execute_sql(self.SQL))
350349
self._check_row_data(rows)
351350

352351

@@ -1085,15 +1084,17 @@ def setUpClass(cls):
10851084

10861085
def _verify_one_column(self, table_desc):
10871086
sql = 'SELECT chunk_me FROM {}'.format(table_desc.table)
1088-
rows = list(self._db.execute_sql(sql))
1087+
with self._db.snapshot() as snapshot:
1088+
rows = list(snapshot.execute_sql(sql))
10891089
self.assertEqual(len(rows), table_desc.row_count)
10901090
expected = table_desc.value()
10911091
for row in rows:
10921092
self.assertEqual(row[0], expected)
10931093

10941094
def _verify_two_columns(self, table_desc):
10951095
sql = 'SELECT chunk_me, chunk_me_2 FROM {}'.format(table_desc.table)
1096-
rows = list(self._db.execute_sql(sql))
1096+
with self._db.snapshot() as snapshot:
1097+
rows = list(snapshot.execute_sql(sql))
10971098
self.assertEqual(len(rows), table_desc.row_count)
10981099
expected = table_desc.value()
10991100
for row in rows:

‎spanner/tests/unit/test_database.py‎

Lines changed: 0 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -621,21 +621,6 @@ def test_session_factory(self):
621621
self.assertIs(session.session_id, None)
622622
self.assertIs(session._database, database)
623623

624-
def test_execute_sql_defaults(self):
625-
QUERY = 'SELECT * FROM employees'
626-
client = _Client()
627-
instance = _Instance(self.INSTANCE_NAME, client=client)
628-
pool = _Pool()
629-
session = _Session()
630-
pool.put(session)
631-
session._execute_result = []
632-
database = self._make_one(self.DATABASE_ID, instance, pool=pool)
633-
634-
rows = list(database.execute_sql(QUERY))
635-
636-
self.assertEqual(rows, [])
637-
self.assertEqual(session._executed, (QUERY, None, None, None, b''))
638-
639624
def test_run_in_transaction_wo_args(self):
640625
import datetime
641626

@@ -678,38 +663,6 @@ def test_run_in_transaction_w_args(self):
678663
self.assertEqual(session._retried,
679664
(_unit_of_work, (SINCE,), {'until': UNTIL}))
680665

681-
def test_read(self):
682-
from google.cloud.spanner.keyset import KeySet
683-
684-
TABLE_NAME = 'citizens'
685-
COLUMNS = ['email', 'first_name', 'last_name', 'age']
686-
KEYS = ['[email protected]', '[email protected]']
687-
KEYSET = KeySet(keys=KEYS)
688-
INDEX = 'email-address-index'
689-
LIMIT = 20
690-
TOKEN = b'DEADBEEF'
691-
client = _Client()
692-
instance = _Instance(self.INSTANCE_NAME, client=client)
693-
pool = _Pool()
694-
session = _Session()
695-
pool.put(session)
696-
database = self._make_one(self.DATABASE_ID, instance, pool=pool)
697-
698-
rows = list(database.read(
699-
TABLE_NAME, COLUMNS, KEYSET, INDEX, LIMIT, TOKEN))
700-
701-
self.assertEqual(rows, [])
702-
703-
(table, columns, key_set, index, limit,
704-
resume_token) = session._read_with
705-
706-
self.assertEqual(table, TABLE_NAME)
707-
self.assertEqual(columns, COLUMNS)
708-
self.assertEqual(key_set, KEYSET)
709-
self.assertEqual(index, INDEX)
710-
self.assertEqual(limit, LIMIT)
711-
self.assertEqual(resume_token, TOKEN)
712-
713666
def test_batch(self):
714667
from google.cloud.spanner.database import BatchCheckout
715668

@@ -951,18 +904,10 @@ def __init__(self, database=None, name=_BaseTest.SESSION_NAME):
951904
self._database = database
952905
self.name = name
953906

954-
def execute_sql(self, sql, params, param_types, query_mode, resume_token):
955-
self._executed = (sql, params, param_types, query_mode, resume_token)
956-
return iter(self._rows)
957-
958907
def run_in_transaction(self, func, *args, **kw):
959908
self._retried = (func, args, kw)
960909
return self._committed
961910

962-
def read(self, table, columns, keyset, index, limit, resume_token):
963-
self._read_with = (table, columns, keyset, index, limit, resume_token)
964-
return iter(self._rows)
965-
966911

967912
class _SessionPB(object):
968913
name = TestDatabase.SESSION_NAME

0 commit comments

Comments
 (0)