Except on sqlite read-only errors with guidance

cytotable/convert.py

@@ -53,7 +53,11 @@ def _get_table_columns_and_types(
 
     import duckdb
 
-    from cytotable.utils import _duckdb_reader, _sqlite_mixed_type_query_to_parquet
+    from cytotable.utils import (
+        _duckdb_reader,
+        _raise_if_sqlite_readonly_error,
+        _sqlite_mixed_type_query_to_parquet,
+    )
 
     source_path = source["source_path"]
     source_type = str(source_path.suffix).lower()
@@ -113,6 +117,8 @@ def _get_table_columns_and_types(
             )
 
     except duckdb.Error as e:
+        _raise_if_sqlite_readonly_error(e, source_path=source_path)
+
         # if we see a mismatched type error
         # run a more nuanced query through sqlite
         # to handle the mixed types
@@ -320,6 +326,12 @@ def _get_table_keyset_pagination_sets(
     Returns:
         Union[List[Optional[Tuple[Union[int, float], Union[int, float]]]], List[None], None]:
             List of keys to use for reading the data later on.
+
+    Raises:
+        duckdb.Error:
+            Re-raised (or converted to SQLiteReadOnlyException) when the
+            underlying duckdb query fails for a reason other than mixed-type
+            or invalid-input errors on a sqlite source.
     """
 
     import logging
@@ -329,7 +341,11 @@ def _get_table_keyset_pagination_sets(
     import duckdb
 
     from cytotable.exceptions import NoInputDataException
-    from cytotable.utils import _duckdb_reader, _generate_pagesets
+    from cytotable.utils import (
+        _duckdb_reader,
+        _generate_pagesets,
+        _raise_if_sqlite_readonly_error,
+    )
 
     logger = logging.getLogger(__name__)
 
@@ -373,12 +389,21 @@ def _get_table_keyset_pagination_sets(
             duckdb.InvalidInputException,
             NoInputDataException,
         ) as invalid_input_exc:
+            if isinstance(invalid_input_exc, duckdb.InvalidInputException):
+                _raise_if_sqlite_readonly_error(
+                    invalid_input_exc, source_path=source_path
+                )
+
             logger.warning(
                 msg=f"Skipping file due to input file errors: {str(invalid_input_exc)}"
             )
 
             return None
 
+        except duckdb.Error as e:
+            _raise_if_sqlite_readonly_error(e, source_path=source_path)
+            raise
+
     elif sql_stmt is not None:
         with _duckdb_reader() as ddb_reader:
             sql_query = f"SELECT {page_key} FROM ({sql_stmt}) ORDER BY {page_key}"
@@ -436,6 +461,7 @@ def _source_pageset_to_parquet(
     from cytotable.utils import (
         _duckdb_reader,
         _extract_npz_to_parquet,
+        _raise_if_sqlite_readonly_error,
         _sqlite_mixed_type_query_to_parquet,
         _write_parquet_table_with_metadata,
     )
@@ -536,6 +562,8 @@ def _source_pageset_to_parquet(
     # Include exception handling to read mixed-type data
     # using sqlite3 and special utility function.
     except duckdb.Error as e:
+        _raise_if_sqlite_readonly_error(e, source_path=source["source_path"])
+
         # if we see a mismatched type error
         # run a more nuanced query through sqlite
         # to handle the mixed types

cytotable/exceptions.py

@@ -26,3 +26,12 @@ class SchemaException(CytoTableException):
     """
     Exception for schema challenges.
     """
+
+
+class SQLiteReadOnlyException(CytoTableException):
+    """
+    Exception for when a SQLite source cannot be opened due to it
+    being in WAL journal mode without write access to its directory
+    (SQLite requires the ability to create '-wal'/'-shm' companion
+    files even for read-only queries against a WAL-mode database).
+    """

cytotable/sources.py

@@ -70,15 +70,24 @@ def _get_source_filepaths(
             since at least one is required to identify source files.
         NoInputDataException:
             Raised when no input files are found at ``path``.
+        duckdb.Error:
+            Re-raised (or converted to SQLiteReadOnlyException) when querying
+            a sqlite source's table list fails for a reason other than the
+            known WAL/readonly failure mode.
     """
 
     import os
     import pathlib
 
+    import duckdb
     from cloudpathlib import AnyPath, CloudPath
 
     from cytotable.exceptions import DatatypeException, NoInputDataException
-    from cytotable.utils import _cache_cloudpath_to_local, _duckdb_reader
+    from cytotable.utils import (
+        _cache_cloudpath_to_local,
+        _duckdb_reader,
+        _raise_if_sqlite_readonly_error,
+    )
 
     if (targets is None or targets == []) and source_datatype is None:
         raise DatatypeException(
@@ -136,6 +145,27 @@ def _get_source_filepaths(
         for element in sources:
             # check that the path is of sqlite type
             if element["source_path"].suffix.lower() == ".sqlite":
+                # perform a query to find the table names from the sqlite file
+                try:
+                    table_names = (
+                        ddb_reader.execute(
+                            """
+                            /* perform query on sqlite_master table for metadata on tables */
+                            SELECT name as table_name
+                            FROM sqlite_scan(?, 'sqlite_master')
+                            WHERE type='table'
+                            """,
+                            parameters=[str(element["source_path"])],
+                        )
+                        .fetch_arrow_table()["table_name"]
+                        .to_pylist()
+                    )
+                except duckdb.Error as e:
+                    _raise_if_sqlite_readonly_error(
+                        e, source_path=element["source_path"]
+                    )
+                    raise
+
                 # creates individual entries for each table
                 expanded_sources += [
                     {
@@ -144,18 +174,7 @@ def _get_source_filepaths(
                         ),
                         "table_name": table_name,
                     }
-                    # perform a query to find the table names from the sqlite file
-                    for table_name in ddb_reader.execute(
-                        """
-                        /* perform query on sqlite_master table for metadata on tables */
-                        SELECT name as table_name
-                        FROM sqlite_scan(?, 'sqlite_master')
-                        WHERE type='table'
-                        """,
-                        parameters=[str(element["source_path"])],
-                    )
-                    .fetch_arrow_table()["table_name"]
-                    .to_pylist()
+                    for table_name in table_names
                     # make sure the table names match with compartment + metadata names
                     if targets is not None
                     and any(target.lower() in table_name.lower() for target in targets)

cytotable/utils.py

@@ -212,6 +212,63 @@ def _duckdb_reader() -> duckdb.DuckDBPyConnection:
     )
 
 
+def _raise_if_sqlite_readonly_error(
+    duckdb_exc: duckdb.Error, source_path: Union[str, pathlib.Path]
+) -> None:
+    """
+    Detects the "attempt to write a readonly database" error which
+    DuckDB's sqlite_scanner raises when a SQLite source is in WAL
+    journal mode and lacks write access to its directory (SQLite
+    requires the ability to create '-wal'/'-shm' companion files even
+    for read-only queries against a WAL-mode database, so this is not
+    avoidable via read-only connection settings). When detected, this
+    raises a more actionable SQLiteReadOnlyException with a suggested
+    fix; otherwise it returns without raising, leaving the original
+    exception for the caller to handle.
+
+    Args:
+        duckdb_exc (duckdb.Error):
+            The duckdb error raised while scanning a SQLite source.
+        source_path (Union[str, pathlib.Path]):
+            Path to the SQLite source which triggered the error.
+
+    Raises:
+        SQLiteReadOnlyException:
+            When the underlying error matches the known WAL/readonly
+            failure mode.
+    """
+
+    import shlex
+
+    from cytotable.exceptions import SQLiteReadOnlyException
+
+    if "attempt to write a readonly database" not in str(duckdb_exc).lower():
+        return
+
+    source_path_arg = shlex.quote(str(source_path))
+
+    raise SQLiteReadOnlyException(
+        f"Unable to read SQLite source '{source_path}' because it appears to be in "
+        "WAL journal mode without write access to its directory. SQLite requires "
+        "the ability to create '-wal'/'-shm' companion files even for read-only "
+        "queries against a WAL-mode database, so this cannot be resolved through "
+        "read-only connection settings alone. This commonly happens when a .sqlite "
+        "file is copied without its '-wal'/'-shm' companion files, or is accessed "
+        "from read-only storage.\n\n"
+        "To fix this, run the following once on a system where you have write "
+        "access to the file, then retry:\n\n"
+        f"    sqlite3 {source_path_arg} 'PRAGMA journal_mode=DELETE;'\n\n"
+        "This checkpoints the database out of WAL mode permanently. This is a safe "
+        "change to make: DELETE is SQLite's original and still most widely used "
+        "journal mode, it does not alter or remove any data in the database, and it "
+        "does not require the database to be closed or otherwise unavailable "
+        "beforehand. The only trade-off is that DELETE mode does not allow readers "
+        "and writers to access the database simultaneously (unlike WAL mode), which "
+        "is not a concern for a database that is being read from as a static "
+        "source, as is the case here."
+    ) from duckdb_exc
+
+
 def _sqlite_mixed_type_query_to_parquet(
     source_path: str,
     table_name: str,

tests/test_convert.py

@@ -10,6 +10,9 @@
 import itertools
 import os
 import pathlib
+import shutil
+import sqlite3
+from contextlib import closing
 from importlib.util import find_spec
 from shutil import copy
 from typing import Any, Dict, List, Tuple, cast
@@ -28,14 +31,15 @@
 from cytotable.convert import (
     _concat_join_sources,
     _concat_source_group,
+    _get_table_keyset_pagination_sets,
     _infer_source_group_common_schema,
     _join_source_pageset,
     _prepare_join_sql,
     _prepend_column_name,
     _run_export_workflow,
     convert,
 )
-from cytotable.exceptions import CytoTableException
+from cytotable.exceptions import CytoTableException, SQLiteReadOnlyException
 from cytotable.presets import config
 from cytotable.sources import _infer_source_datatype
 from cytotable.utils import (
@@ -862,6 +866,46 @@ def test_infer_source_datatype(
         _infer_source_datatype(sources=data)
 
 
+def test_get_table_keyset_pagination_sets_with_wal_mode_readonly_sqlite(
+    load_parsl_default: None, fx_tempdir: str
+):
+    """
+    Tests that pagination raises SQLiteReadOnlyException for a read-only
+    WAL-mode sqlite source missing its '-wal'/'-shm' companion files.
+    """
+
+    tmp_dir_path = pathlib.Path(fx_tempdir)
+
+    source_dir = tmp_dir_path / "pagination_wal_source"
+    source_dir.mkdir()
+    source_path = source_dir / "example.sqlite"
+    with closing(sqlite3.connect(source_path)) as conn:
+        with conn:
+            conn.execute("PRAGMA journal_mode=WAL;")
+            conn.execute("CREATE TABLE Image (ImageNumber INTEGER);")
+            conn.execute("INSERT INTO Image VALUES (1);")
+            conn.commit()
+
+    readonly_dir = tmp_dir_path / "pagination_readonly"
+    readonly_dir.mkdir()
+    readonly_path = readonly_dir / "example.sqlite"
+    shutil.copy(source_path, readonly_path)
+
+    os.chmod(readonly_path, 0o444)
+    os.chmod(readonly_dir, 0o555)
+
+    try:
+        with pytest.raises(SQLiteReadOnlyException, match="journal_mode=DELETE"):
+            _get_table_keyset_pagination_sets(
+                chunk_size=1000,
+                page_key="ImageNumber",
+                source={"table_name": "Image", "source_path": readonly_path},
+            ).result()
+    finally:
+        os.chmod(readonly_dir, 0o755)
+        os.chmod(readonly_path, 0o644)
+
+
 def test_run_export_workflow(
     load_parsl_default: None,
     fx_tempdir: str,

tests/test_sources.py

@@ -2,9 +2,16 @@
 Testing for cytotable/sources.py
 """
 
+import os
 import pathlib
+import shutil
+import sqlite3
 import tempfile
+from contextlib import closing
 
+import pytest
+
+from cytotable.exceptions import SQLiteReadOnlyException
 from cytotable.sources import _file_is_more_than_one_line, _get_source_filepaths
 
 
@@ -67,3 +74,66 @@ def test_get_source_filepaths_with_npz():
             "test_file.npz" in str(file["source_path"])
             for file in result[next(iter(result))]
         )
+
+
+def test_get_source_filepaths_with_wal_mode_readonly_sqlite():
+    """
+    Tests that _get_source_filepaths raises a helpful SQLiteReadOnlyException
+    (instead of an opaque duckdb.Error) when a source .sqlite file is left in
+    WAL journal mode and is missing its '-wal'/'-shm' companion files while
+    the file and its directory are read-only. This reproduces the "attempt to
+    write a readonly database" error which SQLite raises even for read-only
+    queries against such a file.
+
+    This is a meaningful regression check, not a vacuous one: without the
+    os.chmod calls below to make the file/directory read-only, no exception
+    is raised at all (SQLite is able to create the missing '-wal'/'-shm'
+    files on the fly), and if _raise_if_sqlite_readonly_error stops
+    converting the underlying duckdb.Error, this test fails because a plain
+    duckdb.Error (not a SQLiteReadOnlyException) propagates instead.
+
+    Note this test is skipped when running as root, since root can bypass
+    the read-only file permission bits set below, which would prevent the
+    "attempt to write a readonly database" error from being reproduced.
+    """
+    if hasattr(os, "geteuid") and os.geteuid() == 0:
+        pytest.skip(
+            "root bypasses file permission bits, so read-only setup below"
+            " would not reproduce the target error."
+        )
+
+    with tempfile.TemporaryDirectory() as tmp_dir:
+        tmp_dir_path = pathlib.Path(tmp_dir)
+
+        # create a WAL-mode sqlite source with a single table
+        source_dir = tmp_dir_path / "source"
+        source_dir.mkdir()
+        source_path = source_dir / "example.sqlite"
+        with closing(sqlite3.connect(source_path)) as conn:
+            with conn:
+                conn.execute("PRAGMA journal_mode=WAL;")
+                conn.execute("CREATE TABLE Image (ImageNumber INTEGER);")
+                conn.execute("INSERT INTO Image VALUES (1);")
+                conn.commit()
+
+        # copy only the main db file (omitting -wal/-shm companions),
+        # simulating a copy/sync which dropped the companion files
+        readonly_dir = tmp_dir_path / "readonly"
+        readonly_dir.mkdir()
+        readonly_path = readonly_dir / "example.sqlite"
+        shutil.copy(source_path, readonly_path)
+
+        # make the file and its directory read-only
+        os.chmod(readonly_path, 0o444)
+        os.chmod(readonly_dir, 0o555)
+
+        try:
+            with pytest.raises(SQLiteReadOnlyException, match="journal_mode=DELETE"):
+                _get_source_filepaths(
+                    path=readonly_dir,
+                    targets=["image"],
+                )
+        finally:
+            # restore write permissions so TemporaryDirectory cleanup succeeds
+            os.chmod(readonly_dir, 0o755)
+            os.chmod(readonly_path, 0o644)