Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove left over references in geometry columns #139

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
37b2e98
Implement cleaning up schematisation
margrietpalm Nov 15, 2024
0c03d1f
update changes
margrietpalm Nov 20, 2024
ba00a1b
Move cleaning to seperate function
margrietpalm Nov 20, 2024
416b0f5
Remove columns referencing v2 in geometry_column
margrietpalm Nov 21, 2024
1e91d6e
Update changes
margrietpalm Nov 21, 2024
e38d72c
Try to remove tables iwth DropGeoTable
margrietpalm Nov 21, 2024
41f3007
Replace plain drop table with DropGeoTable in migration 0223
margrietpalm Nov 21, 2024
5bf4446
Replace plain drop table with DropGeoTable in migration 0224
margrietpalm Nov 21, 2024
31856f5
Replace plain drop table with DropGeoTable in migration 0225
margrietpalm Nov 21, 2024
5908b72
Replace plain drop table with DropGeoTable in migration 0226
margrietpalm Nov 21, 2024
6a02b55
Replace plain drop table with DropTable in migration 0228 and limit r…
margrietpalm Nov 21, 2024
92a48fb
Move code for dropping tables to single file to reduce duplications
margrietpalm Nov 22, 2024
32e57de
Use spatialite RenameTable to rename table with geometry columns
margrietpalm Nov 22, 2024
6b64d12
Enable removing stuff and clean up triggers
margrietpalm Nov 22, 2024
29ee655
Merge branch 'margriet_schema_300_leftovers' into margriet_107_left_o…
margrietpalm Nov 22, 2024
c27aaa7
Modify drop_geo_table to work with DropGeoTable instead of DropTable …
margrietpalm Nov 22, 2024
c5b6502
Merge branch 'margriet_107_left_over_references_in_geometry_columns' …
margrietpalm Nov 22, 2024
2d76c90
Modify table renaming to work with spatialite 4 and 5
margrietpalm Nov 22, 2024
01fd353
Remove python 3.8 test
margrietpalm Nov 25, 2024
5b1959f
Move to ubuntu 22.04 for python 3.9 test
margrietpalm Nov 25, 2024
2abe679
Merge branch 'margriet_schema_300_leftovers' into margriet_107_left_o…
margrietpalm Nov 25, 2024
5913495
Merge branch 'margriet_schema_300_leftovers' into margriet_107_left_o…
margrietpalm Nov 26, 2024
64d21c5
Make linter happy
margrietpalm Nov 26, 2024
b1577ba
Use DropTable instead of DropGeoTable
margrietpalm Nov 26, 2024
e1c3cab
Remove commented breakpoint
margrietpalm Nov 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 1 addition & 5 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,9 @@ jobs:
fail-fast: false
matrix:
include:
# 2019
- python: 3.8
os: ubuntu-20.04
pins: "sqlalchemy==1.4.44 alembic==1.8.* geoalchemy2==0.14.0"
# 2021
- python: 3.9
os: ubuntu-20.04
os: ubuntu-22.04
pins: "sqlalchemy==1.4.44 alembic==1.8.* geoalchemy2==0.14.0"
# 2022
- python: "3.10"
Expand Down
1 change: 1 addition & 0 deletions CHANGES.rst
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ Changelog of threedi-schema
--------------------

- Remove indices referring to removed tables in previous migrations
- Remove columns referencing v2 in geometry_column



Expand Down
32 changes: 32 additions & 0 deletions threedi_schema/migrations/utils.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
from typing import List

import sqlalchemy as sa


def drop_geo_table(op, table_name: str):
"""

Safely drop table, taking into account geometry columns

Parameters:
op : object
An object representing the database operation.
table_name : str
The name of the table to be dropped.
"""
op.execute(sa.text(f"SELECT DropTable(NULL, '{table_name}');"))


def drop_conflicting(op, new_tables: List[str]):
"""
Drop tables from database that conflict with new tables

Parameters:
op: The SQLAlchemy operation context to interact with the database.
new_tables: A list of new table names to be checked for conflicts with existing tables.
"""
connection = op.get_bind()
existing_tables = [item[0] for item in connection.execute(
sa.text("SELECT name FROM sqlite_master WHERE type='table';")).fetchall()]
for table_name in set(existing_tables).intersection(new_tables):
drop_geo_table(op, table_name)
Comment on lines +1 to +32
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hope this is the right place to put this. I may move other functions here to prevent changing the same functionality multiple times in migrations 222 to 228.

10 changes: 3 additions & 7 deletions threedi_schema/migrations/versions/0222_upgrade_db_settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
from sqlalchemy import Boolean, Column, Float, Integer, String
from sqlalchemy.orm import declarative_base

from threedi_schema.migrations.utils import drop_conflicting

# revision identifiers, used by Alembic.
revision = "0222"
down_revision = "0221"
Expand Down Expand Up @@ -369,18 +371,12 @@ def set_flow_variable_values():
op.execute(sa.text(query))


def drop_conflicting():
new_tables = list(ADD_TABLES.keys()) + [new_name for _, new_name in RENAME_TABLES]
for table_name in new_tables:
op.execute(f"DROP TABLE IF EXISTS {table_name};")


def upgrade():
op.get_bind()
# Only use first row of global settings
delete_all_but_first_row("v2_global_settings")
# Remove existing tables (outside of the specs) that conflict with new table names
drop_conflicting()
drop_conflicting(op, list(ADD_TABLES.keys()) + [new_name for _, new_name in RENAME_TABLES])
rename_tables(RENAME_TABLES)
# rename columns in renamed tables
for table_name, columns in RENAME_COLUMNS.items():
Expand Down
11 changes: 3 additions & 8 deletions threedi_schema/migrations/versions/0223_upgrade_db_inflow.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from threedi_schema.application.threedi_database import load_spatialite
from threedi_schema.domain.custom_types import Geometry
from threedi_schema.migrations.utils import drop_conflicting, drop_geo_table

# revision identifiers, used by Alembic.
revision = "0223"
Expand Down Expand Up @@ -136,7 +137,7 @@ def add_geometry_column(table: str, geocol: Column):

def remove_tables(tables: List[str]):
for table in tables:
op.drop_table(table)
drop_geo_table(op, table)


def copy_values_to_new_table(src_table: str, src_columns: List[str], dst_table: str, dst_columns: List[str]):
Expand Down Expand Up @@ -484,17 +485,11 @@ def fix_geometry_columns():
op.execute(sa.text(migration_query))


def drop_conflicting():
new_tables = list(ADD_TABLES.keys()) + [new_name for _, new_name in RENAME_TABLES]
for table_name in new_tables:
op.execute(f"DROP TABLE IF EXISTS {table_name};")


def upgrade():
connection = op.get_bind()
listen(connection.engine, "connect", load_spatialite)
# Remove existing tables (outside of the specs) that conflict with new table names
drop_conflicting()
drop_conflicting(op, list(ADD_TABLES.keys()) + [new_name for _, new_name in RENAME_TABLES])
# create new tables and rename existing tables
create_new_tables(ADD_TABLES)
rename_tables(RENAME_TABLES)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
from sqlalchemy.orm import declarative_base

from threedi_schema.domain.custom_types import Geometry
from threedi_schema.migrations.utils import drop_conflicting, drop_geo_table

# revision identifiers, used by Alembic.
revision = "0224"
Expand Down Expand Up @@ -335,7 +336,7 @@ def remove_column_from_table(table_name: str, column: str):

def remove_tables(tables: List[str]):
for table in tables:
op.drop_table(table)
drop_geo_table(op, table)


def make_geom_col_notnull(table_name):
Expand All @@ -357,7 +358,7 @@ def make_geom_col_notnull(table_name):
temp_name = f'_temp_224_{uuid.uuid4().hex}'
op.execute(sa.text(f"CREATE TABLE {temp_name} ({','.join(cols)});"))
op.execute(sa.text(f"INSERT INTO {temp_name} ({','.join(col_names)}) SELECT {','.join(col_names)} FROM {table_name}"))
op.execute(sa.text(f"DROP TABLE {table_name};"))
drop_geo_table(op, table_name)
op.execute(sa.text(f"ALTER TABLE {temp_name} RENAME TO {table_name};"))


Expand All @@ -370,15 +371,9 @@ def fix_geometry_columns():
op.execute(sa.text(migration_query))


def drop_conflicting():
new_tables = list(ADD_TABLES.keys()) + [new_name for _, new_name in RENAME_TABLES]
for table_name in new_tables:
op.execute(f"DROP TABLE IF EXISTS {table_name};")


def upgrade():
# Remove existing tables (outside of the specs) that conflict with new table names
drop_conflicting()
drop_conflicting(op, list(ADD_TABLES.keys()) + [new_name for _, new_name in RENAME_TABLES])
# create new tables and rename existing tables
create_new_tables(ADD_TABLES)
rename_tables(RENAME_TABLES)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
from sqlalchemy.orm import declarative_base

from threedi_schema.domain.custom_types import Geometry
from threedi_schema.migrations.utils import drop_conflicting, drop_geo_table

# revision identifiers, used by Alembic.
revision = "0225"
Expand Down Expand Up @@ -119,8 +120,13 @@

def rename_tables(table_sets: List[Tuple[str, str]]):
# no checks for existence are done, this will fail if a source table doesn't exist
connection = op.get_bind()
spatialite_version = connection.execute(sa.text("SELECT spatialite_version();")).fetchall()[0][0]
for src_name, dst_name in table_sets:
op.rename_table(src_name, dst_name)
if spatialite_version.startswith('5'):
op.execute(sa.text(f"SELECT RenameTable(NULL, '{src_name}', '{dst_name}');"))
else:
op.rename_table(src_name, dst_name)


def create_new_tables(new_tables: Dict[str, sa.Column]):
Expand Down Expand Up @@ -183,7 +189,7 @@ def rename_columns(table_name: str, columns: List[Tuple[str, str]]):
create_table_query = f"""CREATE TABLE {temp_name} ({', '.join(new_columns_list_sql_formatted)});"""
op.execute(sa.text(create_table_query))
op.execute(sa.text(f"INSERT INTO {temp_name} ({','.join(new_columns_list)}) SELECT {','.join(old_columns_list)} from {table_name};"))
op.execute(sa.text(f"DROP TABLE {table_name};"))
drop_geo_table(op, table_name)
op.execute(sa.text(f"ALTER TABLE {temp_name} RENAME TO {table_name};"))

for entry in new_columns:
Expand Down Expand Up @@ -215,15 +221,9 @@ def populate_table(table: str, values: dict):
op.execute(sa.text(query))


def drop_conflicting():
new_tables = [new_name for _, new_name in RENAME_TABLES]
for table_name in new_tables:
op.execute(f"DROP TABLE IF EXISTS {table_name};")


def upgrade():
# Drop tables that conflict with new table names
drop_conflicting()
drop_conflicting(op, [new_name for _, new_name in RENAME_TABLES])

# rename existing tables
rename_tables(RENAME_TABLES)
Expand Down
12 changes: 3 additions & 9 deletions threedi_schema/migrations/versions/0226_upgrade_db_1d_1d2d.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
from sqlalchemy import Boolean, Column, Float, Integer, String, Text
from sqlalchemy.orm import declarative_base

from threedi_schema.domain.custom_types import Geometry
from threedi_schema.migrations.utils import drop_conflicting, drop_geo_table

# revision identifiers, used by Alembic.
revision = "0226"
Expand Down Expand Up @@ -74,7 +74,7 @@ def add_columns_to_tables(table_columns: List[Tuple[str, Column]]):

def remove_tables(tables: List[str]):
for table in tables:
op.drop_table(table)
drop_geo_table(op, table)


def modify_table(old_table_name, new_table_name):
Expand Down Expand Up @@ -167,15 +167,9 @@ def set_potential_breach_final_exchange_level():
))


def drop_conflicting():
new_tables = [new_name for _, new_name in RENAME_TABLES]
for table_name in new_tables:
op.execute(f"DROP TABLE IF EXISTS {table_name};")


def upgrade():
# Drop tables that conflict with new table names
drop_conflicting()
drop_conflicting(op, [new_name for _, new_name in RENAME_TABLES])
rem_tables = []
for old_table_name, new_table_name in RENAME_TABLES:
modify_table(old_table_name, new_table_name)
Expand Down
14 changes: 3 additions & 11 deletions threedi_schema/migrations/versions/0228_upgrade_db_1D.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

from threedi_schema.domain import constants, models
from threedi_schema.domain.custom_types import IntegerEnum
from threedi_schema.migrations.utils import drop_conflicting, drop_geo_table

Base = declarative_base()

Expand Down Expand Up @@ -99,7 +100,7 @@ def add_columns_to_tables(table_columns: List[Tuple[str, Column]]):

def remove_tables(tables: List[str]):
for table in tables:
op.drop_table(table)
drop_geo_table(op, table)


def modify_table(old_table_name, new_table_name):
Expand Down Expand Up @@ -439,21 +440,12 @@ def fix_material_id():
f"{' '.join([f'WHEN {old} THEN {new}' for old, new in replace_map.items()])} "
"ELSE material_id END"))



def drop_conflicting():
new_tables = [new_name for _, new_name in RENAME_TABLES] + ['material', 'pump_map']
for table_name in new_tables:
op.execute(f"DROP TABLE IF EXISTS {table_name};")



def upgrade():
# Empty or non-existing connection node id (start or end) in Orifice, Pipe, Pumpstation or Weir will break
# migration, so an error is raised in these cases
check_for_null_geoms()
# Prevent custom tables in schematisation from breaking migration when they conflict with new table names
drop_conflicting()
drop_conflicting(op, [new_name for _, new_name in RENAME_TABLES] + ['material', 'pump_map'])
# Extent cross section definition table (actually stored in temp)
extend_cross_section_definition_table()
# Migrate data from cross_section_definition to cross_section_location
Expand Down
32 changes: 27 additions & 5 deletions threedi_schema/migrations/versions/0229_clean_up.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
Create Date: 2024-11-15 14:18

"""

from typing import List

import sqlalchemy as sa
Expand All @@ -20,23 +19,46 @@

def remove_tables(tables: List[str]):
for table in tables:
op.drop_table(table)

op.drop_table(table)


def find_tables_by_pattern(pattern: str) -> List[str]:
connection = op.get_bind()
query = connection.execute(sa.text(f"select name from sqlite_master where type = 'table' and name like '{pattern}'"))
query = connection.execute(
sa.text(f"select name from sqlite_master where type = 'table' and name like '{pattern}'"))
return [item[0] for item in query.fetchall()]


def remove_old_tables():
remaining_v2_idx_tables = find_tables_by_pattern('idx_v2_%_the_geom')
remaining_alembic = find_tables_by_pattern('%_alembic_%_the_geom')
remove_tables(remaining_v2_idx_tables+remaining_alembic)
remove_tables(remaining_v2_idx_tables + remaining_alembic)


def clean_geometry_columns():
""" Remove columns referencing v2 in geometry_columns """
op.execute(sa.text("""
DELETE FROM geometry_columns WHERE f_table_name IN (
SELECT g.f_table_name FROM geometry_columns g
LEFT JOIN sqlite_master m ON g.f_table_name = m.name
WHERE m.name IS NULL AND g.f_table_name like "%v2%"
);
"""))


def clean_triggers():
""" Remove triggers referencing v2 tables """
connection = op.get_bind()
triggers = [item[0] for item in connection.execute(
sa.text("SELECT tbl_name FROM sqlite_master WHERE type='trigger' AND tbl_name LIKE '%v2%';")).fetchall()]
for trigger in triggers:
op.execute(f"DROP TRIGGER IF EXISTS {trigger};")


def upgrade():
remove_old_tables()
clean_geometry_columns()
clean_triggers()


def downgrade():
Expand Down