Merge pull request #3975 from mathesar-foundation/remove_dead_code_2

Remove dead code 2 (the code strikes back)

conftest.py

@@ -7,15 +7,14 @@
 # These imports come from the mathesar namespace, because our DB setup logic depends on it.
 from django.db import connection as dj_connection
 
-from sqlalchemy import MetaData, text, Table
+from sqlalchemy import MetaData, text, Table, select, or_
 from sqlalchemy.exc import OperationalError
 from sqlalchemy_utils import database_exists, create_database, drop_database
 
 from db.engine import add_custom_types_to_ischema_names, create_engine as sa_create_engine
 from db.sql import install as sql_install
-from db.schemas.operations.drop import drop_schema_via_name as drop_sa_schema
-from db.schemas.operations.create import create_schema_if_not_exists_via_sql_alchemy
-from db.schemas.utils import get_schema_oid_from_name, get_schema_name_from_oid
+from db.utils import get_pg_catalog_table
+from db.metadata import get_empty_metadata
 
 from fixtures.utils import create_scoped_fixtures
 
@@ -162,28 +161,6 @@ def _test_schema_name():
     return "_test_schema_name"
 
 
-# TODO does testing this make sense?
-@pytest.fixture(scope="module")
-def engine_without_ischema_names_updated(test_db_name, MOD_engine_cache):
-    """
-    For testing environments where an engine might not be fully setup.
-
-    We instantiate a new engine cache, without updating its ischema_names dict.
-    """
-    return MOD_engine_cache(test_db_name)
-
-
-# TODO seems unneeded: remove
-@pytest.fixture
-def engine_with_schema_without_ischema_names_updated(
-    engine_without_ischema_names_updated, _test_schema_name, create_db_schema
-):
-    engine = engine_without_ischema_names_updated
-    schema_name = _test_schema_name
-    create_db_schema(schema_name, engine)
-    return engine, schema_name
-
-
 @pytest.fixture
 def engine_with_schema(engine, _test_schema_name, create_db_schema):
     schema_name = _test_schema_name
@@ -210,8 +187,8 @@ def _create_schema(schema_name, engine, schema_mustnt_exist=True):
         if schema_mustnt_exist:
             assert schema_name not in created_schemas
         logger.debug(f'creating {schema_name}')
-        create_schema_if_not_exists_via_sql_alchemy(schema_name, engine)
-        schema_oid = get_schema_oid_from_name(schema_name, engine)
+        _create_schema_if_not_exists_via_sql_alchemy(schema_name, engine)
+        schema_oid = _get_schema_oid_from_name(schema_name, engine)
         db_name = engine.url.database
         created_schemas_in_this_engine = created_schemas.setdefault(db_name, {})
         created_schemas_in_this_engine[schema_name] = schema_oid
@@ -223,15 +200,74 @@ def _create_schema(schema_name, engine, schema_mustnt_exist=True):
         try:
             for _, schema_oid in created_schemas_in_this_engine.items():
                 # Handle schemas being renamed during test
-                schema_name = get_schema_name_from_oid(schema_oid, engine)
+                schema_name = _get_schema_name_from_oid(schema_oid, engine)
                 if schema_name:
-                    drop_sa_schema(engine, schema_name, cascade=True)
+                    _drop_schema_via_name(engine, schema_name, cascade=True)
                     logger.debug(f'dropping {schema_name}')
         except OperationalError as e:
             logger.debug(f'ignoring operational error: {e}')
     logger.debug('exit')
 
 
+def _create_schema_if_not_exists_via_sql_alchemy(schema_name, engine):
+    return _execute_msar_func_with_engine(
+        engine, 'create_schema_if_not_exists', schema_name
+    ).fetchone()[0]
+
+
+def _execute_msar_func_with_engine(engine, func_name, *args):
+    """
+    Execute an msar function using an SQLAlchemy engine.
+
+    This is temporary scaffolding.
+
+    Args:
+        engine: an SQLAlchemy engine for connecting to a DB
+        func_name: The unqualified msar function name (danger; not sanitized)
+        *args: The list of parameters to pass
+    """
+    conn_str = str(engine.url)
+    with psycopg.connect(conn_str) as conn:
+        return conn.execute(
+            f"SELECT msar.{func_name}({','.join(['%s'] * len(args))})",
+            args
+        )
+
+
+def _get_schema_name_from_oid(oid, engine, metadata=None):
+    schema_info = _reflect_schema(engine, oid=oid, metadata=metadata)
+    if schema_info:
+        return schema_info["name"]
+
+
+def _get_schema_oid_from_name(name, engine):
+    schema_info = _reflect_schema(engine, name=name)
+    if schema_info:
+        return schema_info["oid"]
+
+
+def _reflect_schema(engine, name=None, oid=None, metadata=None):
+    # If we have both arguments, the behavior is undefined.
+    try:
+        assert name is None or oid is None
+    except AssertionError as e:
+        raise e
+    # TODO reuse metadata
+    metadata = metadata if metadata else get_empty_metadata()
+    pg_namespace = get_pg_catalog_table("pg_namespace", engine, metadata=metadata)
+    sel = (
+        select(pg_namespace.c.oid, pg_namespace.c.nspname.label("name"))
+        .where(or_(pg_namespace.c.nspname == name, pg_namespace.c.oid == oid))
+    )
+    with engine.begin() as conn:
+        schema_info = conn.execute(sel).fetchone()
+    return schema_info
+
+
+def _drop_schema_via_name(engine, name, cascade=False):
+    _execute_msar_func_with_engine(engine, 'drop_schema', name, cascade).fetchone()
+
+
 # Seems to be roughly equivalent to mathesar/database/base.py::create_mathesar_engine
 # TODO consider fixing this seeming duplication
 # either way, both depend on Django configuration. can that be resolved?
@@ -260,9 +296,6 @@ def _get_connection_string(username, password, hostname, database):
 ACADEMICS_SQL = os.path.join(RESOURCES, "academics_create.sql")
 LIBRARY_SQL = os.path.join(RESOURCES, "library_without_checkouts.sql")
 LIBRARY_CHECKOUTS_SQL = os.path.join(RESOURCES, "library_add_checkouts.sql")
-FRAUDULENT_PAYMENTS_SQL = os.path.join(RESOURCES, "fraudulent_payments.sql")
-PLAYER_PROFILES_SQL = os.path.join(RESOURCES, "player_profiles.sql")
-MARATHON_ATHLETES_SQL = os.path.join(RESOURCES, "marathon_athletes.sql")
 
 
 @pytest.fixture
@@ -334,54 +367,3 @@ def make_table(table_name):
         in table_names
     }
     return tables
-
-
-@pytest.fixture
-def engine_with_fraudulent_payment(engine_with_schema):
-    engine, schema = engine_with_schema
-    with engine.begin() as conn, open(FRAUDULENT_PAYMENTS_SQL) as f:
-        conn.execute(text(f"SET search_path={schema}"))
-        conn.execute(text(f.read()))
-    yield engine, schema
-
-
-@pytest.fixture
-def payments_db_table(engine_with_fraudulent_payment):
-    engine, schema = engine_with_fraudulent_payment
-    metadata = MetaData(bind=engine)
-    table = Table("Payments", metadata, schema=schema, autoload_with=engine)
-    return table
-
-
-@pytest.fixture
-def engine_with_player_profiles(engine_with_schema):
-    engine, schema = engine_with_schema
-    with engine.begin() as conn, open(PLAYER_PROFILES_SQL) as f:
-        conn.execute(text(f"SET search_path={schema}"))
-        conn.execute(text(f.read()))
-    yield engine, schema
-
-
-@pytest.fixture
-def players_db_table(engine_with_player_profiles):
-    engine, schema = engine_with_player_profiles
-    metadata = MetaData(bind=engine)
-    table = Table("Players", metadata, schema=schema, autoload_with=engine)
-    return table
-
-
-@pytest.fixture
-def engine_with_marathon_athletes(engine_with_schema):
-    engine, schema = engine_with_schema
-    with engine.begin() as conn, open(MARATHON_ATHLETES_SQL) as f:
-        conn.execute(text(f"SET search_path={schema}"))
-        conn.execute(text(f.read()))
-    yield engine, schema
-
-
-@pytest.fixture
-def athletes_db_table(engine_with_marathon_athletes):
-    engine, schema = engine_with_marathon_athletes
-    metadata = MetaData(bind=engine)
-    table = Table("Marathon", metadata, schema=schema, autoload_with=engine)
-    return table

db/columns/base.py

@@ -1,16 +1,10 @@
+# TODO Remove this file once explorations are in the database
 from sqlalchemy import Column, ForeignKey, inspect
 
-from db.columns.defaults import TYPE, PRIMARY_KEY, NULLABLE, DEFAULT_COLUMNS
-from db.columns.operations.select import (
-    get_column_attnum_from_name, get_column_default, get_column_default_dict,
-)
-from db.tables.operations.select import get_oid_from_table
-from db.types.operations.cast import get_full_cast_map
+from db.columns.operations.select import get_column_attnum_from_name
 from db.types.operations.convert import get_db_type_enum_from_class
 
 
-# TODO consider renaming to DbColumn or DatabaseColumn
-# We are attempting to reserve the term Mathesar for types in the mathesar namespace.
 class MathesarColumn(Column):
     """
     This class constrains the possible arguments, enabling us to include
@@ -102,17 +96,6 @@ def from_column(cls, column, engine=None):
             )
         return new_column
 
-    def to_sa_column(self):
-        """
-        MathesarColumn sometimes is not interchangeable with SQLAlchemy's Column.
-        For use in those situations, this method attempts to recreate an SA Column.
-
-        NOTE: this method is incomplete: it does not account for all properties of MathesarColumn.
-        """
-        sa_column = Column(name=self.name, type_=self.type)
-        sa_column.table = self.table_
-        return sa_column
-
     @property
     def table_(self):
         """
@@ -128,52 +111,16 @@ def table_(self):
     @property
     def table_oid(self):
         if self.table_ is not None:
-            oid = get_oid_from_table(
-                self.table_.name, self.table_.schema, self.engine
+            oid = inspect(self.engine).get_table_oid(
+                self.table_.name, schema=self.table_.schema
             )
         else:
             oid = None
         return oid
 
-    @property
-    def is_default(self):
-        default_def = DEFAULT_COLUMNS.get(self.name, False)
-        try:
-            self.type.python_type
-        except NotImplementedError:
-            return False
-        return (
-            default_def
-            and self.type.python_type == default_def[TYPE]().python_type
-            and self.primary_key == default_def.get(PRIMARY_KEY, False)
-            and self.nullable == default_def.get(NULLABLE, True)
-        )
-
     def add_engine(self, engine):
         self.engine = engine
 
-    @property
-    def valid_target_types(self):
-        """
-        Returns a set of valid types to which the type of the column can be
-        altered.
-        """
-        if (
-            self.engine is not None
-            and not self.is_default
-            and self.db_type is not None
-        ):
-            db_type = self.db_type
-            valid_target_types = sorted(
-                list(
-                    set(
-                        get_full_cast_map(self.engine).get(db_type, [])
-                    )
-                ),
-                key=lambda db_type: db_type.id
-            )
-            return valid_target_types if valid_target_types else []
-
     @property
     def column_attnum(self):
         """
@@ -182,8 +129,6 @@ def column_attnum(self):
         """
         engine_exists = self.engine is not None
         table_exists = self.table_ is not None
-        # TODO are we checking here that the table exists on the database? explain why we have to do
-        # that.
         engine_has_table = inspect(self.engine).has_table(
             self.table_.name,
             schema=self.table_.schema,
@@ -197,31 +142,6 @@ def column_attnum(self):
                 metadata=metadata,
             )
 
-    @property
-    def column_default_dict(self):
-        if self.table_ is None:
-            return
-        metadata = self.table_.metadata
-        default_dict = get_column_default_dict(
-            self.table_oid, self.column_attnum, self.engine, metadata=metadata,
-        )
-        if default_dict:
-            return {
-                'is_dynamic': default_dict['is_dynamic'],
-                'value': default_dict['value']
-            }
-
-    @property
-    def default_value(self):
-        if self.table_ is not None:
-            metadata = self.table_.metadata
-            return get_column_default(
-                self.table_oid,
-                self.column_attnum,
-                self.engine,
-                metadata=metadata,
-            )
-
     @property
     def db_type(self):
         """

db/columns/defaults.py

@@ -1,24 +1,4 @@
-from sqlalchemy import Integer
-
-from db import constants
-
-
 NAME = "name"
 DESCRIPTION = "description"
 NULLABLE = "nullable"
-PRIMARY_KEY = "primary_key"
-TYPE = "sa_type"
 DEFAULT = "default"
-AUTOINCREMENT = "autoincrement"
-
-ID_TYPE = Integer
-
-
-DEFAULT_COLUMNS = {
-    constants.ID: {
-        TYPE: ID_TYPE,
-        PRIMARY_KEY: True,
-        NULLABLE: False,
-        AUTOINCREMENT: True
-    }
-}