Merge branch 'split-datetime-into-three-ui-types' of github.com:cente…

…rofci/mathesar into split-datetime-into-three-ui-types

Dockerfile.integ-tests

@@ -111,7 +111,7 @@ RUN wget https://github.com/jwilder/dockerize/releases/download/$DOCKERIZE_VERSI
     && rm dockerize-linux-amd64-$DOCKERIZE_VERSION.tar.gz
 
 RUN pip install playwright==1.18.2
-RUN pip install pytest-playwright==0.2.2
+RUN pip install pytest-playwright==0.2.3
 RUN playwright install
 RUN playwright install-deps
 

db/columns/operations/alter.py

@@ -6,7 +6,9 @@
 
 from db.columns.defaults import NAME, NULLABLE
 from db.columns.exceptions import InvalidDefaultError, InvalidTypeError, InvalidTypeOptionError
-from db.columns.operations.select import get_column_attnum_from_name, get_column_default, get_column_index_from_name
+from db.columns.operations.select import (
+    get_column_attnum_from_name, get_column_default, get_column_name_from_attnum,
+)
 from db.columns.utils import get_mathesar_column_with_engine, get_type_options
 from db.tables.operations.select import get_oid_from_table, reflect_table_from_oid
 from db.types.base import get_db_type_name
@@ -37,14 +39,16 @@ def alter_column(engine, table_oid, column_index, column_data):
                 table, column_index, engine, conn,
                 type_options=column_data[TYPE_OPTIONS_KEY]
             )
-
+        column_name = table.columns[column_index].name
+        column_attnum = get_column_attnum_from_name(table_oid, column_name, engine)
         if NULLABLE_KEY in column_data:
             nullable = column_data[NULLABLE_KEY]
-            change_column_nullable(table, column_index, engine, conn, nullable)
+            change_column_nullable(table_oid, column_attnum, engine, conn, nullable)
         if DEFAULT_DICT in column_data:
             default_dict = column_data[DEFAULT_DICT]
             default = default_dict[DEFAULT_KEY] if default_dict is not None else None
-            set_column_default(table, column_index, engine, conn, default)
+
+            set_column_default(table_oid, column_attnum, engine, conn, default)
         if NAME_KEY in column_data:
             # Name always needs to be the last item altered
             # since previous operations need the name to work
@@ -72,13 +76,12 @@ def alter_column_type(
     # Re-reflect table so that column is accurate
     table = reflect_table_from_oid(table_oid, engine, connection)
     column = table.columns[column_name]
-    column_index = get_column_index_from_name(table_oid, column_name, engine, connection)
     column_attnum = get_column_attnum_from_name(table_oid, column_name, engine, connection)
 
     default = get_column_default(table_oid, column_attnum, engine, connection)
     if default is not None:
         default_text = column.server_default.arg.text
-    set_column_default(table, column_index, engine, connection, None)
+    set_column_default(table_oid, column_attnum, engine, connection, None)
 
     prepared_table_name = _preparer.format_table(table)
     prepared_column_name = _preparer.format_column(column)
@@ -97,7 +100,7 @@ def alter_column_type(
         cast_stmt = f"{cast_function_name}({default_text})"
         default_stmt = select(text(cast_stmt))
         new_default = str(execute_statement(engine, default_stmt, connection).first()[0])
-        set_column_default(table, column_index, engine, connection, new_default)
+        set_column_default(table_oid, column_attnum, engine, connection, new_default)
 
 
 def retype_column(
@@ -135,15 +138,19 @@ def retype_column(
         raise e.orig
 
 
-def change_column_nullable(table, column_index, engine, connection, nullable):
-    column = table.columns[column_index]
+def change_column_nullable(table_oid, column_attum, engine, connection, nullable):
+    table = reflect_table_from_oid(table_oid, engine, connection)
+    column_name = get_column_name_from_attnum(table_oid, column_attum, engine)
+    column = table.columns[column_name]
     ctx = MigrationContext.configure(connection)
     op = Operations(ctx)
     op.alter_column(table.name, column.name, nullable=nullable, schema=table.schema)
 
 
-def set_column_default(table, column_index, engine, connection, default):
-    column = table.columns[column_index]
+def set_column_default(table_oid, column_attnum, engine, connection, default):
+    table = reflect_table_from_oid(table_oid, engine, connection)
+    column_name = get_column_name_from_attnum(table_oid, column_attnum, engine)
+    column = table.columns[column_name]
     default_clause = DefaultClause(str(default)) if default is not None else default
     try:
         ctx = MigrationContext.configure(connection)

db/columns/operations/create.py

@@ -9,7 +9,9 @@
 from db.columns.defaults import DEFAULT, NAME, NULLABLE, TYPE
 from db.columns.exceptions import InvalidDefaultError, InvalidTypeError, InvalidTypeOptionError
 from db.columns.operations.alter import set_column_default, change_column_nullable
-from db.columns.operations.select import get_column_attnum_from_name, get_column_default, get_column_index_from_name
+from db.columns.operations.select import (
+    get_column_attnum_from_name, get_column_default, get_column_name_from_attnum,
+)
 from db.columns.utils import get_mathesar_column_with_engine
 from db.constraints.operations.create import copy_constraint
 from db.constraints.operations.select import get_column_constraints
@@ -92,43 +94,45 @@ def compile_copy_column(element, compiler, **_):
     )
 
 
-def _duplicate_column_data(table_oid, from_column, to_column, engine):
+def _duplicate_column_data(table_oid, from_column_attnum, to_column_attnum, engine):
     table = reflect_table_from_oid(table_oid, engine)
-    from_column_attnum = get_column_attnum_from_name(table_oid, table.c[from_column].name, engine)
+    from_column_name = get_column_name_from_attnum(table_oid, from_column_attnum, engine)
+    to_column_name = get_column_name_from_attnum(table_oid, to_column_attnum, engine)
     copy = CopyColumn(
         table.schema,
         table.name,
-        table.c[to_column].name,
-        table.c[from_column].name
+        to_column_name,
+        from_column_name,
     )
     with engine.begin() as conn:
         conn.execute(copy)
     from_default = get_column_default(table_oid, from_column_attnum, engine)
     if from_default is not None:
         with engine.begin() as conn:
-            set_column_default(table, to_column, engine, conn, from_default)
+            set_column_default(table_oid, to_column_attnum, engine, conn, from_default)
 
 
-def _duplicate_column_constraints(table_oid, from_column, to_column, engine, copy_nullable=True):
+def _duplicate_column_constraints(table_oid, from_column_attnum, to_column_attnum, engine, copy_nullable=True):
     table = reflect_table_from_oid(table_oid, engine)
+    from_column_name = get_column_name_from_attnum(table_oid, from_column_attnum, engine)
     if copy_nullable:
         with engine.begin() as conn:
-            change_column_nullable(table, to_column, engine, conn, table.c[from_column].nullable)
-
-    constraints = get_column_constraints(from_column, table_oid, engine)
+            change_column_nullable(table_oid, to_column_attnum, engine, conn, table.c[from_column_name].nullable)
+    constraints = get_column_constraints(from_column_attnum, table_oid, engine)
     for constraint in constraints:
         constraint_type = constraint_utils.get_constraint_type_from_char(constraint.contype)
         if constraint_type != constraint_utils.ConstraintType.UNIQUE.value:
             # Don't allow duplication of primary keys
             continue
         copy_constraint(
-            table, engine, constraint, from_column, to_column
+            table_oid, engine, constraint, from_column_attnum, to_column_attnum
         )
 
 
-def duplicate_column(table_oid, copy_from_index, engine, new_column_name=None, copy_data=True, copy_constraints=True):
+def duplicate_column(table_oid, copy_from_attnum, engine, new_column_name=None, copy_data=True, copy_constraints=True):
     table = reflect_table_from_oid(table_oid, engine)
-    from_column = table.c[copy_from_index]
+    copy_from_name = get_column_name_from_attnum(table_oid, copy_from_attnum, engine)
+    from_column = table.c[copy_from_name]
     if new_column_name is None:
         new_column_name = _gen_col_name(table, from_column.name)
 
@@ -138,25 +142,24 @@ def duplicate_column(table_oid, copy_from_index, engine, new_column_name=None, c
         NULLABLE: True,
     }
     new_column = create_column(engine, table_oid, column_data)
-    new_column_index = get_column_index_from_name(table_oid, new_column.name, engine)
-
+    new_column_attnum = get_column_attnum_from_name(table_oid, new_column.name, engine)
     if copy_data:
         _duplicate_column_data(
             table_oid,
-            copy_from_index,
-            new_column_index,
+            copy_from_attnum,
+            new_column_attnum,
             engine
         )
 
     if copy_constraints:
         _duplicate_column_constraints(
             table_oid,
-            copy_from_index,
-            new_column_index,
+            copy_from_attnum,
+            new_column_attnum,
             engine,
             copy_nullable=copy_data
         )
 
     table = reflect_table_from_oid(table_oid, engine)
-    column_index = get_column_index_from_name(table_oid, new_column_name, engine)
-    return get_mathesar_column_with_engine(table.c[column_index], engine)
+    column_name = get_column_name_from_attnum(table_oid, new_column_attnum, engine)
+    return get_mathesar_column_with_engine(table.c[column_name], engine)

db/columns/operations/select.py

@@ -83,18 +83,33 @@ def get_column_indexes_from_table(table_oid, engine, connection_to_use=None):
     return results
 
 
-def get_column_name_from_attnum(table_oid, attnum, engine, connection_to_use=None):
+def _get_columns_name_from_attnums(table_oid, attnums, engine, connection_to_use=None):
     with warnings.catch_warnings():
         warnings.filterwarnings("ignore", message="Did not recognize type")
         pg_attribute = Table("pg_attribute", MetaData(), autoload_with=engine)
     sel = select(pg_attribute.c.attname).where(
         and_(
             pg_attribute.c.attrelid == table_oid,
-            pg_attribute.c.attnum == attnum
+            pg_attribute.c.attnum.in_(attnums)
         )
     )
-    result = execute_statement(engine, sel, connection_to_use).scalar()
-    return result
+    return sel
+
+
+def get_columns_name_from_attnums(table_oid, attnums, engine, connection_to_use=None):
+    """
+    Returns the respective list of attnum of the column names passed.
+     The order is based on the column order in the table and not by the order of the column names argument.
+    """
+    statement = _get_columns_name_from_attnums(table_oid, attnums, engine, connection_to_use=None)
+    column_names_tuple = execute_statement(engine, statement, connection_to_use).fetchall()
+    column_names = [column_name_tuple[0] for column_name_tuple in column_names_tuple]
+    return column_names
+
+
+def get_column_name_from_attnum(table_oid, attnum, engine, connection_to_use=None):
+    statement = _get_columns_name_from_attnums(table_oid, [attnum], engine, connection_to_use=None)
+    return execute_statement(engine, statement, connection_to_use).scalar()
 
 
 def get_column_default_dict(table_oid, attnum, engine, connection_to_use=None):

db/constraints/operations/create.py

@@ -2,7 +2,9 @@
 from alembic.operations import Operations
 from sqlalchemy import MetaData
 
+from db.columns.operations.select import get_columns_name_from_attnums
 from db.constraints.utils import get_constraint_type_from_char, ConstraintType, naming_convention
+from db.tables.operations.select import reflect_table_from_oid
 
 
 def create_unique_constraint(table_name, schema, engine, columns, constraint_name=None):
@@ -16,14 +18,13 @@ def create_unique_constraint(table_name, schema, engine, columns, constraint_nam
         op.create_unique_constraint(constraint_name, table_name, columns, schema)
 
 
-def copy_constraint(table, engine, constraint, from_column, to_column):
+def copy_constraint(table_oid, engine, constraint, from_column_attnum, to_column_attnum):
+    table = reflect_table_from_oid(table_oid, engine)
     constraint_type = get_constraint_type_from_char(constraint.contype)
     if constraint_type == ConstraintType.UNIQUE.value:
-        column_idxs = [con - 1 for con in constraint.conkey]
-        columns = [
-            table.c[to_column if idx == from_column else idx].name
-            for idx in column_idxs
-        ]
+        column_attnums = constraint.conkey
+        changed_column_attnums = [to_column_attnum if attnum == from_column_attnum else attnum for attnum in column_attnums]
+        columns = get_columns_name_from_attnums(table_oid, changed_column_attnums, engine)
         create_unique_constraint(table.name, table.schema, engine, columns)
     else:
         raise NotImplementedError

db/constraints/operations/select.py

@@ -49,7 +49,7 @@ def get_constraint_oid_by_name_and_table_oid(name, table_oid, engine):
     return result['oid']
 
 
-def get_column_constraints(column_index, table_oid, engine):
+def get_column_constraints(column_attnum, table_oid, engine):
     metadata = MetaData()
     with warnings.catch_warnings():
         warnings.filterwarnings("ignore", message="Did not recognize type")
@@ -62,7 +62,7 @@ def get_column_constraints(column_index, table_oid, engine):
             pg_constraint.c.conrelid == table_oid,
             # 'conkey' contains a list of the constrained column's indices
             # Here, we check if the column index appears in the conkey list
-            pg_constraint.c.conkey.bool_op("&&")(f"{{{column_index + 1}}}")
+            pg_constraint.c.conkey.bool_op("&&")(f"{{{column_attnum}}}")
         ))
     )
 

db/tests/columns/operations/test_alter.py

@@ -262,10 +262,12 @@ def test_change_column_nullable_changes(engine_with_schema, nullable_tup):
         Column(nontarget_column_name, String),
     )
     table.create()
+    table_oid = get_oid_from_table(table_name, schema, engine)
+    target_column_attnum = get_column_attnum_from_name(table_oid, target_column_name, engine)
     with engine.begin() as conn:
         change_column_nullable(
-            table,
-            0,
+            table_oid,
+            target_column_attnum,
             engine,
             conn,
             nullable_tup[1],
@@ -289,6 +291,7 @@ def test_change_column_nullable_with_data(engine_with_schema, nullable_tup):
         Column(target_column_name, Integer, nullable=nullable_tup[0]),
     )
     table.create()
+    table_oid = get_oid_from_table(table_name, schema, engine)
     ins = table.insert().values(
         [
             {target_column_name: 1},
@@ -298,10 +301,11 @@ def test_change_column_nullable_with_data(engine_with_schema, nullable_tup):
     )
     with engine.begin() as conn:
         conn.execute(ins)
+    target_column_attnum = get_column_attnum_from_name(table_oid, target_column_name, engine)
     with engine.begin() as conn:
         change_column_nullable(
-            table,
-            0,
+            table_oid,
+            target_column_attnum,
             engine,
             conn,
             nullable_tup[1],
@@ -324,6 +328,8 @@ def test_change_column_nullable_changes_raises_with_null_data(engine_with_schema
         Column(target_column_name, Integer, nullable=True),
     )
     table.create()
+    table_oid = get_oid_from_table(table_name, schema, engine)
+    target_column_attnum = get_column_attnum_from_name(table_oid, target_column_name, engine)
     ins = table.insert().values(
         [
             {target_column_name: 1},
@@ -336,8 +342,8 @@ def test_change_column_nullable_changes_raises_with_null_data(engine_with_schema
     with engine.begin() as conn:
         with pytest.raises(IntegrityError) as e:
             change_column_nullable(
-                table,
-                0,
+                table_oid,
+                target_column_attnum,
                 engine,
                 conn,
                 False,
@@ -360,7 +366,7 @@ def test_column_default_create(engine_with_schema, col_type):
     table_oid = get_oid_from_table(table_name, schema, engine)
     column_attnum = get_column_attnum_from_name(table_oid, column_name, engine)
     with engine.begin() as conn:
-        set_column_default(table, 0, engine, conn, set_default)
+        set_column_default(table_oid, column_attnum, engine, conn, set_default)
 
     default = get_column_default(table_oid, column_attnum, engine)
     created_default = get_default(engine, table)
@@ -384,7 +390,7 @@ def test_column_default_update(engine_with_schema, col_type):
     table_oid = get_oid_from_table(table_name, schema, engine)
     column_attnum = get_column_attnum_from_name(table_oid, column_name, engine)
     with engine.begin() as conn:
-        set_column_default(table, 0, engine, conn, set_default)
+        set_column_default(table_oid, column_attnum, engine, conn, set_default)
     default = get_column_default(table_oid, column_attnum, engine)
     created_default = get_default(engine, table)
 
@@ -408,7 +414,7 @@ def test_column_default_delete(engine_with_schema, col_type):
     table_oid = get_oid_from_table(table_name, schema, engine)
     column_attnum = get_column_attnum_from_name(table_oid, column_name, engine)
     with engine.begin() as conn:
-        set_column_default(table, 0, engine, conn, None)
+        set_column_default(table_oid, column_attnum, engine, conn, None)
     default = get_column_default(table_oid, column_attnum, engine)
     created_default = get_default(engine, table)