kvesteri / sqlalchemy-utils

Various utility functions and datatypes for SQLAlchemy.
Other
1.27k stars 321 forks source link

view.create_view() causes DuplicateTableError on Base.metadata.create_all(checkfirst=True) #396

Open willjp opened 5 years ago

willjp commented 5 years ago

Using Base.metadata.create_all() with a view raises a sqlalchemy.exc.ProgrammingError if the view already exists. Occurs in both sqlite, and postgresql - if that is helpful.

class TeamMemberView(Base):
    _query = sqlalchemy.select([Table1, Table2])
    __tablename__ = 'my_view'
    __table__ = sqlalchemy_utils.create_view(__tablename__, _query, Base.metadata)

# followed by
Base.metadata.create_all(engine, checkfirst=True)

This workflow is not in the official documentation, if I am not using this as intended please let me know, and I'm sorry :)

willjp commented 5 years ago

if it is helpful for others, my workaround is

try:
    Base.metadata.create_all(engine, checkfirst=True)
except(sqlalchemy.orm.OperationalError, sqlalchemy.orm.ProgrammingError):
    for table in Base.metadata.tables:
        Base.metadata.tables[table].create(bind=engine, checkfirst=True)
bjrnfrdnnd commented 4 years ago

Hi willjp, I ran into the same problem. Your workaround works - but only for tables. No views are being created in my case. It seems that calling create_all() is necessary to create views in the database - I do not know of any workaround to create different sets of views by calling create_all twice in the same program: The second time create_all() is called, it throws the abovementioned error and no views are being created. The workaround then works - but only to create tables, no views.

willjp commented 4 years ago

Thank you very much for letting me know! I'll try to figure something else out, and I'll repost if I can come up with something. Good luck!

RenaKunisaki commented 4 years ago

Here's a workaround:

from sqlalchemy_utils.view import CreateView
from sqlalchemy.ext import compiler

@compiler.compiles(CreateView)
def compile_create_materialized_view(element, compiler, **kw):
    return 'CREATE OR REPLACE {}VIEW {} AS {}'.format(
        'MATERIALIZED ' if element.materialized else '',
        element.name,
        compiler.sql_compiler.process(element.selectable, literal_binds=True),
)

The correct fix would be to add OR REPLACE to the SQL in the actual view.py file, but this works to hotpatch it.

volkerjaenisch commented 4 years ago

I think the problem here is a bug in view.py (Line 155)

    table = create_table_from_selectable(
        name=name,
        selectable=selectable,
        metadata=None
    )

metadata=None should be read metadata=metadata

This bug prevents the view to be registered in the correct metadata therefore leading to follow up errors, e.g. with create_all, drop_all.

Cheers,

Volker

moomoohk commented 4 years ago

Hi

@volkerjaenisch, could you please specify here as well that your assumption was incorrect?

@RenaKunisaki, your solution unfortunately won't work with SQLite and probably a few other dialects... The view would have to be dropped beforehand. Best way I was able to do it:

from sqlalchemy_utils.view import DropView

DropView(ViewClass.__table__.name, cascade=do_i_want_to_cascade)(ViewClass.__table__, engine)
Base.metadata.create_all(engine)

Unfortunately ViewClass.__table__.drop(engine) and Base.metadata.drop_all(engine, [ViewClass.__table__]) raised errors (tried to run DROP TABLE on the view for some reason, which obviously fails)

OscartGiles commented 4 years ago

Here's a workaround:

from sqlalchemy_utils.view import CreateView
from sqlalchemy.ext import compiler

@compiler.compiles(CreateView)
def compile_create_materialized_view(element, compiler, **kw):
    return 'CREATE OR REPLACE {}VIEW {} AS {}'.format(
        'MATERIALIZED ' if element.materialized else '',
        element.name,
        compiler.sql_compiler.process(element.selectable, literal_binds=True),
)

The correct fix would be to add OR REPLACE to the SQL in the actual view.py file, but this works to hotpatch it.

For a materialized view you might want to do:

@compiler.compiles(CreateView)
def compile_create_materialized_view(element, compiler, **kw):
    return "CREATE {}VIEW IF NOT EXISTS {} AS {}".format(
        "MATERIALIZED " if element.materialized else "",
        element.name,
        compiler.sql_compiler.process(element.selectable, literal_binds=True),
    )
claudiopastorini commented 4 years ago

Ok, it is work, but I do not really understand why to "hack" the code of a library in this way.

@kvesteri, why the compile_create_materialized_view() does not use the IF NOT EXISTS statement? Do we miss something or this is intended to be as it is?

talhajunaidd commented 3 years ago

Here's a workaround:

from sqlalchemy_utils.view import CreateView
from sqlalchemy.ext import compiler

@compiler.compiles(CreateView)
def compile_create_materialized_view(element, compiler, **kw):
    return 'CREATE OR REPLACE {}VIEW {} AS {}'.format(
        'MATERIALIZED ' if element.materialized else '',
        element.name,
        compiler.sql_compiler.process(element.selectable, literal_binds=True),
)

The correct fix would be to add OR REPLACE to the SQL in the actual view.py file, but this works to hotpatch it.

For a materialized view you might want to do:

@compiler.compiles(CreateView)
def compile_create_materialized_view(element, compiler, **kw):
    return "CREATE {}VIEW IF NOT EXISTS {} AS {}".format(
        "MATERIALIZED " if element.materialized else "",
        element.name,
        compiler.sql_compiler.process(element.selectable, literal_binds=True),
    )

This cause problem with indexes

kpflugshaupt commented 3 years ago

For what it's worth, here is my workaround for this. (I'm using non-materialized-views only here, these can always be dropped and recreated without any loss).

VIEWS: Dict[str, Table] = {
    "bookings_with_alloc": create_view(
        "bookings_with_alloc", selectable=bookings_with_alloc, metadata=_metadata
    ),
    "timeslots_with_alloc": create_view(
        "timeslots_with_alloc", selectable=timeslots_with_alloc, metadata=_metadata
    ),
}

# always drop views before creating them (create_view() does not...)
with DYM_DB.begin() as conn:
    for view_name in VIEWS:
        conn.execute(f"drop view if exists {view_name}")

_metadata.create_all(DYM_DB)

DYM_DB is my DB engine (PostgreSQL in this case). This only works on databases supporting "drop view if exists".

andersmd7 commented 2 years ago

For what it's worth, here is my workaround for this. (I'm using non-materialized-views only here, these can always be dropped and recreated without any loss).

VIEWS: Dict[str, Table] = {
    "bookings_with_alloc": create_view(
        "bookings_with_alloc", selectable=bookings_with_alloc, metadata=_metadata
    ),
    "timeslots_with_alloc": create_view(
        "timeslots_with_alloc", selectable=timeslots_with_alloc, metadata=_metadata
    ),
}

# always drop views before creating them (create_view() does not...)
with DYM_DB.begin() as conn:
    for view_name in VIEWS:
        conn.execute(f"drop view if exists {view_name}")

_metadata.create_all(DYM_DB)

DYM_DB is my DB engine (PostgreSQL in this case). This only works on databases supporting "drop view if exists".

I would argue it's not true that dynamic views can always be dropped and recreated without any loss. I have a number of use-cases where I have materialized views that select from dynamic views, in PostgreSQL. A simple drop of the dynamic views will need to cascade to the materialized views that reference them. Instead, I can use CREATE OR REPLACE VIEW to replace the dynamic view, without messing with the materialized view. The materialized views obviously need to be refreshed after replacing the view, but that's generally less troublesome than having to recreate them after they've been dropped. since refreshing doesn't entail recreating indexes, etc.

The "correct fix" mentioned by @RenaKunisaki should address this issue by creating a 1:1 option for the native CREATE OR REPLACE VIEW in PostgreSQL.