Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGES
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,11 @@ In development
- If a model has a table name that matches an existing table in the metadata,
use that table. Fixes a regression where reflected tables were not picked up
by models. (`#551`_)
- Raise the correct error when a model has a table name but no primary key.
(`#556`_)

.. _#551: https://github.com/mitsuhiko/flask-sqlalchemy/pull/551
.. _#556: https://github.com/mitsuhiko/flask-sqlalchemy/pull/556


Version 2.3.0
Expand Down
13 changes: 13 additions & 0 deletions flask_sqlalchemy/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -73,18 +73,31 @@ def __table_cls__(cls, *args, **kwargs):
If no primary key is found, that indicates single-table inheritance,
so no table will be created and ``__tablename__`` will be unset.
"""
# check if a table with this name already exists
# allows reflected tables to be applied to model by name
key = _get_table_key(args[0], kwargs.get('schema'))

if key in cls.metadata.tables:
return sa.Table(*args, **kwargs)

# if a primary key or constraint is found, create a table for
# joined-table inheritance
for arg in args:
if (
(isinstance(arg, sa.Column) and arg.primary_key)
or isinstance(arg, sa.PrimaryKeyConstraint)
):
return sa.Table(*args, **kwargs)

# if no base classes define a table, return one
# ensures the correct error shows up when missing a primary key
for base in cls.__mro__[1:-1]:
if '__table__' in base.__dict__:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd probably go for

if not any('__table__' in b.__dict__ for b in cls.__mro__[1:-1]):
    return sa.Table(*args, **kwargs)

but IMO both are pretty well-readable so no strong opinon ;)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Was doing that initially for the check above this, but it looked too complicated. Kept it the same here for consistency.

break
else:
return sa.Table(*args, **kwargs)

# single-table inheritance, use the parent tablename
if '__tablename__' in cls.__dict__:
del cls.__tablename__

Expand Down
10 changes: 10 additions & 0 deletions tests/test_table_name.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import inspect

import pytest
from sqlalchemy.exc import ArgumentError
from sqlalchemy.ext.declarative import declared_attr


Expand Down Expand Up @@ -203,3 +205,11 @@ class User(db.Model):
pass

assert User.__table__ is user


def test_correct_error_for_no_primary_key(db):
with pytest.raises(ArgumentError) as info:
class User(db.Model):
pass

assert 'could not assemble any primary key' in str(info.value)