Skip to content

Commit

Permalink
Fix Column dropping operations to use attnum instead of column index
Browse files Browse the repository at this point in the history
  • Loading branch information
silentninja committed Feb 28, 2022
1 parent 1ae3b0c commit f7029fb
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 7 deletions.
7 changes: 4 additions & 3 deletions db/columns/operations/drop.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
from alembic.migration import MigrationContext
from alembic.operations import Operations

from db.columns.operations.select import get_column_name_from_attnum
from db.tables.operations.select import reflect_table_from_oid


def drop_column(table_oid, column_index, engine):
column_index = int(column_index)
def drop_column(table_oid, column_attnum, engine):
table = reflect_table_from_oid(table_oid, engine)
column = table.columns[column_index]
column_name = get_column_name_from_attnum(table_oid, column_attnum, engine)
column = table.columns[column_name]
with engine.begin() as conn:
ctx = MigrationContext.configure(conn)
op = Operations(ctx)
Expand Down
4 changes: 3 additions & 1 deletion db/tests/columns/operations/test_drop.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from sqlalchemy import String, Integer, Column, Table, MetaData

from db.columns.operations.drop import drop_column
from db.columns.operations.select import get_column_attnum_from_name
from db.tables.operations.select import get_oid_from_table, reflect_table_from_oid
from db.tests.types import fixtures

Expand All @@ -23,7 +24,8 @@ def test_drop_column_correct_column(engine_with_schema):
)
table.create()
table_oid = get_oid_from_table(table_name, schema, engine)
drop_column(table_oid, 0, engine)
column_attnum = get_column_attnum_from_name(table_oid, target_column_name, engine)
drop_column(table_oid, column_attnum, engine)
altered_table = reflect_table_from_oid(table_oid, engine)
assert len(altered_table.columns) == 1
assert nontarget_column_name in altered_table.columns
Expand Down
2 changes: 1 addition & 1 deletion mathesar/api/db/viewsets/columns.py
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ def destroy(self, request, pk=None, table_pk=None):
column_instance = self.get_object()
table = column_instance.table
try:
table.drop_column(column_instance.column_index)
table.drop_column(column_instance.attnum)
column_instance.delete()
except IndexError:
raise NotFound
Expand Down
4 changes: 2 additions & 2 deletions mathesar/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,10 +238,10 @@ def alter_column(self, column_index, column_data):
column_data,
)

def drop_column(self, column_index):
def drop_column(self, column_attnum):
drop_column(
self.oid,
column_index,
column_attnum,
self.schema._sa_engine,
)

Expand Down

0 comments on commit f7029fb

Please sign in to comment.