Fix #26 Use unique MetaData object for each bind

[ghost]

An InvalidRequestError occurs when subclassing db.Model with two objects
that have the same __tablename__ but different __bind_key__.

sqlalchemy.exc.InvalidRequestError: Table 'users' is already defined for
this MetaData instance.  Specify 'extend_existing=True' to redefine
options and columns on an existing Table object.

This could happen, for example, if you had a users table in two
different databases that you needed to access. This patch creates a
unique MetaData object for each bind_key which stops this error from
happening and allows access to tables with the same name in different
databases. It also includes a test to verify the error is
fixed.

[jonathanq]

I would love to see this merged. Is there anything left to do in order to merge it?

[davidism]

We need to consider how this will fit with #255 and other issues related to more flexible metadata and base models.

[ghost]

Unfortunately, passing a custom MetaData object to the __init__ method (like #255) will not fix or affect this bug. If #255 is merged you would need to decide how the custom MetaData object would affect the binds:

1. It does not affect the binds. The binds use a default MetaData object. (No changes to PR required)
2. The new MetaData objects created for the binds take on the optional parameters of the custom MetaData passed to __init__.
3. Allow each bind to accept an optional MetaData object in the SQLALCHEMY_BINDS dictionary.

If you merged this PR and then merged #255 after you would essentially get 1; the main URI would use the custom MetaData object passed to __init__ and the binds would use MetaData objects with default parameters.

If you would prefer options 2 or 3 I'd be more than happy to fix this pull request to get the desired functionality. I would also really like to see this fix get merged as #26 is a really old bug and I'm forced to use a modified version of Flask-SQLAlchemy on a number of my projects due to this bug.

[theY4Kman]

This pull request is also a solution to a show-stopping bug for us. We are using SQLALCHEMY_BINDS to connect to multiple databases with different backends. One of our models has an ENUM type. It seems, though, that because there's only one MetaData instance, it tries to create the ENUM type for every bind. Here's a small example:

from flask import Flask
from flask.ext.sqlalchemy import SQLAlchemy


app = Flask(__name__)
app.config['SQLALCHEMY_BINDS'] = {
    'postgres': 'postgresql+psycopg2://[email protected]/test',
    'mysql': 'mysql://[email protected]/test',
}
db = SQLAlchemy(app)


class PostgresModel(db.Model):
    __tablename__ = 'postgres_model'
    __bind_key__ = 'postgres'

    id = db.Column(db.Integer, primary_key=True)


class MySQLModel(db.Model):
    __tablename__ = 'mysql_model'
    __bind_key__ = 'mysql'

    id = db.Column(db.Integer, primary_key=True)
    my_enum = db.Column(db.Enum('A', 'B'))


db.create_all()

Executing this script will give you the error:

sqlalchemy.exc.CompileError: Postgresql ENUM type requires a name.

I installed the Flask-SQLAlchemy proposed in this pull request, and all was fine.

[TehMillhouse]

I'd love to see this merged as well (or a variant of it). Any update on this?

[dylanjbarth]

This would be a major upgrade for our use case as well, anything we can do to help get this merged?

[rissoa]

any progress of this merge? I have same object name in two db with binds. Now, it tells me:
sqlalchemy.exc.InvalidRequestError: Table 'products' is already defined for this MetaData instance. Specify 'extend_existing=True' to redefine options and columns on an existing Table object.

[RonnyPfannschmidt]

IMHO this is a hack adding breakage on top of a wrong assumption

if one needs objects of different databases potentially sharing the same identity key one should always use a different session object to begin with instead of just declaring the same table twice and throwing things together in some way

[rissoa]

@RonnyPfannschmidt Does flask-sqlalchemy support multiple sessions? I can not find any document about it. how it deal with db config?

I know I can get my answer by reading code..

[RonnyPfannschmidt]

You can use them manually, but its not explicitly supported and integrated

[kblomqvist]

I started building a webapp using Flask a month ago and now I'm facing this same issue. I think that the patch is not a hack. It does what I expect to happen when using binds.

[ghost]

I have updated and slightly improved this pull request on top of the current codebase. It seems like this is causing issues for a lot of people. Previously, there was a concern how this pull request would fit with other open pull requests that were also making changes to metadata but they've all been merged into the project now and none of them fixed this issue. It would be really nice to get this merged so we can stop having to keep modified flask-sqlalchemy library in our projects' repos.

[davidism]

I definitely want to get this in 3.0. There are a lot of customization things that have built up, and it's hard to keep track of them all and make sure they all form a consistent api. If any of you have input on #357 (subclassing vs __init__ vs attributes), that would be appreciated.

• I feel like this could be closely related to Manage external declarative bases #250 (external bases). I'm not sure if either could or should be modified to work together more closely.
• This doesn't use the custom metadata from Add custom meta data parameter #255, as has already been somewhat discussed. It seems wrong to throw away the custom metadata for the binds, but it seems verbose to require defining a separate custom one for each.

[kblomqvist]

Could we use copy/deepcopy here for not throwing customized metadata away?

self.Model._metadata[bind] = copy(self.metadata)

[kblomqvist]

I don't know much about the Flask and its extensions' internals as I just started. For me the __init__ type of customization feels the most convenient for now. However, I think that this issue is only slightly related to the customization? The main issue is that binds don't work as expected even without any customization.

[kblomqvist]

So what do you think? Would the copy of metadata class solve the concern of throwing the customized metadata away?

[Windfarer]

So this issue exists for about 3 years, any update to it?

[davidism]

I will have more time to work on this after PyCon; my current focus is Flask. Or during PyCon, if anyone who wants to work on Flask-SQLAlchemy will be at the sprints.

Unfortunately, there are only two maintainers of this project, and we're both volunteering our limited free time.

Conceptually, I'm ok with this, but I need to find time to thoroughly review it and consider how it will interact with other parts of the library and other features that need to get in, as stated above.

[flip111]

Here is another way to trigger this error

error: sqlalchemy.exc.InvalidRequestError: Table 'table_name_here' is already defined for this MetaData instance. Specify 'extend_existing=True' to redefine options and columns on an existing Table object.

class MyModel(db.Model):
    __tablename__ = 'mymodel'
    id = db.Column(db.Integer, primary_key=True)
    anotherModel = db.relationship("AnotherModel")

class MyModelSchema(ma.ModelSchema):
    class Meta:
        model = MyModel

mymodel_schema = MyModelSchema()


class AnotherModel(db.Model):
    __tablename__ = 'anothermodel'
    id = db.Column(db.Integer, primary_key=True)
    mymodel_id = db.Column(db.Integer, db.ForeignKey('mymodel.id'))

If AnotherModel class is placed above MyModel class the problem is fixed.

sorry if this doesn't relate to the PR ... this is where i land when i google for this error ..

[timbess]

Is there any good workaround for this issue for the time being? It'd be really awesome to be able to use SQLAlchemy again and this is the only thing preventing it from working.

EDIT:
A good workaround seems to be calling SQLAlchemy.get_engine(current_app, 'binding') and manually creating a SQLAlchemy Session bound to that engine. Then I can just create one model and use it with different Sessions. Would that cause any weird concurrency issues though? Don't wanna dig too far into the internals and mess it up.

[davidism]

While this was a pretty clever solution, it doesn't handle _decl_class_registry (SQLAlchemy <= 1.3), or registry in 1.4, which can still result in overlapping names. With 1.4, replacing registry seems even more complex. I like the 1.4 API, but if we want to support 1.3 for some time we now have to support two implementations. After reviewing this code as well as how things work in SQLAlchemy, I've decided that I can't merge this. I've opened #941 to explain what I've found and to discuss further ideas.

[davidism]

#1087