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

Improve reflection mechanism and alembic interactions #374

Merged

Conversation

adrien-berchet
Copy link
Member

@adrien-berchet adrien-berchet commented Mar 13, 2022

Goals of this PR:

  • keep the spatial index creation easy (with spatial_index=True) and compatible with all SQLAlchemy mechanisms and with Alembic.
  • fix the reflection mechanism for spatial types and spatial indexes.
  • fix Issue with PostGIS spatial index using 0.11 #376

Main issues:

  • spatial indexes are properly reflected with Postgres but not with SQLIte (because they are tables).
  • indexes are reflected after the columns so it is not possible to know during column processing if we should create an index or let it be reflected afterwards.
  • using _column_flag=True makes it work with to_metadata but it's a private attribute so it might not be reliable?
  • Alembic triggers the before_create and after_create events but not the before_drop and after_drop events on which we rely on.

@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch 2 times, most recently from 251adf0 to be6b69e Compare March 14, 2022 16:48
@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch 3 times, most recently from 52cc395 to 297d6ce Compare March 14, 2022 18:10
The _column_flag attribute is used to skip index creation in the to_metadata()
method.
@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch 2 times, most recently from 9a2db94 to 58de466 Compare May 13, 2022 16:11
@adrien-berchet adrien-berchet changed the title Fix reflection for SpatiaLite columns Improve reflection mechanism and alembic interactions May 13, 2022
@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch from 58de466 to 84bb0d0 Compare May 13, 2022 16:19
@adrien-berchet
Copy link
Member Author

Hi @zzzeek
Here is the following of sqlalchemy/alembic#1034.
As I said, I still have (at least) one issue with Alembic: the CreateTableOp operation triggers the before_create and after_create events as expected but the DropTableOp does not trigger the before_drop and after_drop events on which we rely on to properly drop the spatial columns and to clean the remaining indexes with SQLite. Is there any reason for that or was it forgotten? I can create an issue in Alembic for this.

@zzzeek
Copy link
Contributor

zzzeek commented May 14, 2022

the before/after create dispatch was added to support some ENUM use cases which relied upon these to create their constraints. it looks like the current scope of before/after create/drop is limited to just MetaData and Table in SQLAlchemy (like, not for constraints or anything like that) so sure if you want to send a PR w/ tests for the drop events, that's fine, look right here:

https://github.com/sqlalchemy/alembic/blob/188dd8b2f098e804c8e64956e9c6490d41f1d7ce/alembic/ddl/impl.py#L350-L374

@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch from 15825b6 to 75dfba9 Compare May 31, 2022 08:59
@adrien-berchet adrien-berchet marked this pull request as ready for review May 31, 2022 08:59
@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch from 1c9fd41 to 219f259 Compare June 1, 2022 20:59
@adrien-berchet adrien-berchet force-pushed the fix_sqlite_reflection branch from 80f201f to 5deed2a Compare June 2, 2022 08:45
@adrien-berchet adrien-berchet merged commit 5922af1 into geoalchemy:master Jun 2, 2022
@adrien-berchet adrien-berchet deleted the fix_sqlite_reflection branch June 2, 2022 08:57
@adrien-berchet adrien-berchet linked an issue Jun 2, 2022 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Issue with PostGIS spatial index using 0.11 The new RecoverGeometryColumn breaks alembic migrations
2 participants