DanCardin / sqlalchemy-declarative-extensions

Library to declare additional kinds of objects not natively supported by SqlAlchemy/Alembic.
https://sqlalchemy-declarative-extensions.readthedocs.io/en/latest/
Apache License 2.0
36 stars 7 forks source link

feature request: add WITH NO DATA to the @view #60

Closed veledzimovich-iTechArt closed 5 months ago

veledzimovich-iTechArt commented 5 months ago

It would be nice to have the possibility to provide the WITH NO DATA directive as a keyword argument to the @view decorator from sqlalchemy_declarative_extensions.

Because right now, we have to add it manually in our migration scripts.

op.execute(
        """
        CREATE MATERIALIZED VIEW my_view AS
        SELECT set_config('timezone', 'UTC', TRUE)  -- set timezone to UTC
        WITH NO DATA  -- do not populate the data during the migration [MANUALLY ADDED DIRECTIVE]
        """ 
)
DanCardin commented 5 months ago

I'd already realized that the current interface implies too much commonality among dialects. As such my PR begins to remove the concept of materialized views from the generic base version.

However, you mentioned the @view decorator, so I would expect your usage to remain unchanged. As you'll see in the PR, the current interface looks like @view(..., materialized=MaterializedOptions(with_data=False)) or @view(..., materialized={"with_data":False})

DanCardin commented 5 months ago

I'd appreciate it if you could test the PR out ahead of my merging it, to make sure it does what you're anticipating it do.

veledzimovich-iTechArt commented 5 months ago

Thank you!

I made some tests. Here are the process and results.

  1. I define MyView.
    
    set_timezone_cte = select(text("set_config('timezone', 'UTC', TRUE)")).cte(
    "set_timezone"
    )

@view(Base, materialized=MaterializedOptions(with_data=False)) class MyView: tablename = "my_view" view = select(set_timezone_cte, literal(True))

2.  I use `alembic` to generate migration.

```bash
alembic revision --autogenerate
  1. I got the expected migration.
def upgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.execute(
        """
        CREATE MATERIALIZED VIEW my_view AS
        WITH set_timezone AS (
            SELECT SET_CONFIG('timezone', 'UTC', TRUE)
        )
        SELECT TRUE AS anon_1 FROM set_timezone
        WITH NO DATA;
        """
    )
    # ### end Alembic commands ###

def downgrade() -> None:
    # ### commands auto generated by Alembic - please adjust! ###
    op.execute("""DROP MATERIALIZED VIEW my_view""")
    # ### end Alembic commands ###
  1. alembic check passed successfully.

I confirm that if I use MaterializedOptions(with_data=False), the migration is generated correctly.

I have small question. Could you confirm that this is correct?

If I delete WITH NO DATA from the migration script and migrate the database, I expect:

DanCardin commented 5 months ago

I'd have to double check the code, but i wouldn't expect it to. There might be a user introspectable marker for whether it has been populated yet, but i haven't hooked it up if there is.

My expectation of how this feature should work is that it's a transient state of the view, not a persistent attribute of it, so it ought to only affect the initial creation.

Does that sound right?

veledzimovich-iTechArt commented 5 months ago

I talked to team mate about that and he think that WITH NO DATA is not the persistent attribute too. Thank you.

veledzimovich-iTechArt commented 5 months ago

I confirm that everything work as expected @DanCardin