Document (and reconsider design of) Database.execute() and Database.execute_against_connection_in_thread()

[simonw]

In #683 I started a new section of internals documentation covering the Database class: https://datasette.readthedocs.io/en/latest/internals.html#database-class

I decided not to document .execute() and .execute_against_connection_in_thread() yet because I'm not 100% happy with their API design yet.

[simonw]

Current implementations:

datasette/datasette/database.py

Lines 103 to 168 in a093c5f

	async def execute_against_connection_in_thread(self, fn):
	def in_thread():
	conn = getattr(connections, self.name, None)
	if not conn:
	conn = self.connect()
	self.ds.prepare_connection(conn, self.name)
	setattr(connections, self.name, conn)
	return fn(conn)
	return await asyncio.get_event_loop().run_in_executor(
	self.ds.executor, in_thread
	)
	async def execute(
	self,
	sql,
	params=None,
	truncate=False,
	custom_time_limit=None,
	page_size=None,
	log_sql_errors=True,
	):
	"""Executes sql against db_name in a thread"""
	page_size = page_size or self.ds.page_size
	def sql_operation_in_thread(conn):
	time_limit_ms = self.ds.sql_time_limit_ms
	if custom_time_limit and custom_time_limit < time_limit_ms:
	time_limit_ms = custom_time_limit
	with sqlite_timelimit(conn, time_limit_ms):
	try:
	cursor = conn.cursor()
	cursor.execute(sql, params or {})
	max_returned_rows = self.ds.max_returned_rows
	if max_returned_rows == page_size:
	max_returned_rows += 1
	if max_returned_rows and truncate:
	rows = cursor.fetchmany(max_returned_rows + 1)
	truncated = len(rows) > max_returned_rows
	rows = rows[:max_returned_rows]
	else:
	rows = cursor.fetchall()
	truncated = False
	except (sqlite3.OperationalError, sqlite3.DatabaseError) as e:
	if e.args == ("interrupted",):
	raise QueryInterrupted(e, sql, params)
	if log_sql_errors:
	print(
	"ERROR: conn={}, sql = {}, params = {}: {}".format(
	conn, repr(sql), params, e
	)
	)
	raise
	if truncate:
	return Results(rows, truncated, cursor.description)
	else:
	return Results(rows, False, cursor.description)
	with trace("sql", database=self.name, sql=sql.strip(), params=params):
	results = await self.execute_against_connection_in_thread(
	sql_operation_in_thread
	)
	return results

At the very least the method name execute_against_connection_in_thread() should be updated to something that's more similar to the new (and documented) .execute_write_fn() method.

[simonw]

Working with this in simonw/datasette-media#2 made me really want to redesign this API: simonw/datasette-media@be23ec3

[simonw]

I'm going to rename execute_against_connection_in_thread() to just execute_fn().

[simonw]

The API design for the .execute() function is actually fine, I think it's more the API of the returned Results object that I want to improve.

That object encapsulates the returned data, the named columns and whether or not the result was truncated.

return Results(rows, truncated, cursor.description)

The rows argument comes from either rows = cursor.fetchmany(max_returned_rows + 1) or rows = cursor.fetchall(). It's a Python list of sqlite3.Row objects.

Here's the current class implementation:

datasette/datasette/utils/__init__.py

Lines 54 to 68 in 0784f2e

	class Results:
	def __init__(self, rows, truncated, description):
	self.rows = rows
	self.truncated = truncated
	self.description = description
	@property
	def columns(self):
	return [d[0] for d in self.description]
	def __iter__(self):
	return iter(self.rows)
	def __len__(self):
	return len(self.rows)

[simonw]

Maybe all I really want here is to add length and access-by-index methods to the Results class? So I don't have to do results.rows[0][0] to get at a single returned value.

Also how about a results.single_value() method which returns a value only if there is a single row with a single column, otherwise raises an exception?

[simonw]

I'm going to add .first() and .last() methods too. These will return the first or last row, or None if the results are empty (rather than raising an IndexError).

[simonw]

I'll write the API documentation first, in a branch.

[simonw]

I'm not going to do .last() because it could be confusing if truncation has come into play.

[simonw]

Documentation for improved design: https://github.com/simonw/datasette/pull/763/files#diff-87703da03a02e0f8dae01f39abd6250f

[simonw]

This code should live somewhere other than datasette/utils/__init__.py. Especially the exceptions, since calling code needs to be able to sensibly import them.

[simonw]

I'll move the code into datasette/database.py.

[simonw]

[ Fun aside: I implemented and shipped this entire branch in my browser using the beta of GitHub's new Codespaces ]

[simonw]

New documentation is live here: https://datasette.readthedocs.io/en/latest/internals.html#database-class

[simonw]

I'm going to ship a release with just this change purely so I can start depending on it from my in-development https://github.com/simonw/datasette-media plugin.

[simonw]

Re-opening this ticket because I forgot to document execute_fn() (the old execute_against_connection_in_thread() method).