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

Change sql commands order #36

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Conversation

evstratbg
Copy link

If run peeweedbevolve.evolve from the scratch, wo any table, there is an error, if other tables have foreign keys, cause of commands order create table-alter table-create (unique) index

In this PR I've change the order to create table-create (unique) index-alter table

And some pep8 fixes ;)

If run `peeweedbevolve.evolve` from the scratch, wo any table, there is an error, if other tables have foreign keys, cause of commands order `create table-alter table-create (unique) index`

In this PR I've change the order to `create table-create (unique) index-alter table`
@keredson
Copy link
Owner

can you resubmit w/o the code reformatting? it's impossible to tell what the change is here.

@evstratbg
Copy link
Author

ye, 1 min

@keredson
Copy link
Owner

cool, thanks for the contribution!

so you moved two pieces:

  • create foreign keys
  • rename tables

the latter:

  for k, v in table_renames.items():
    to_run += rename_table(migrator, k, v)

will definitely break things, as that code in the middle is all done to make changes using the new table name instead of the old. (and likely that's the root cause for those test failures.)

the former seems a good idea tho. (i can't think of anything off hand that would have to happen after a FK creation.) is there a test case we could add that would differentiate the behaviors and/or highlight the original error you encountered?

@evstratbg
Copy link
Author

evstratbg commented May 29, 2019

yes, here is sample of models.py to check

import peeweedbevolve
from peewee import (
    CharField, Model, AutoField, ForeignKeyField,
    PostgresqlDatabase
)
from playhouse.shortcuts import model_to_dict


PG_CONN = {
    'host': 'localhost',
    'database': os.environ.get('PG_DATABASE'),
    'user': os.environ.get('PG_USER'),
    'password': os.environ.get('PG_PASSWORD'),
    'autorollback': True,
}

db = PostgresqlDatabase(**PG_CONN)


class BaseModel(Model):

    def as_dict(self, **kwargs):
        return model_to_dict(self, **kwargs)

    def refresh(self):
        return type(self).get(self._pk_expr())

    class Meta:
        database = db


class Users(BaseModel):
    id = AutoField(unique=True)
    uid = CharField(unique=True, index=True)


class Sessions(BaseModel):
    id = AutoField(unique=True)
    user = ForeignKeyField(
        Users,
        to_field='uid',
        on_update='CASCADE',
        column_name='user_uid',
        backref='sessions',
        null=True
    )


if __name__ == '__main__':
    peeweedbevolve.evolve(db, interactive=False, ignore_tables=['basemodel'])

@evstratbg
Copy link
Author

So, no need to move

for k, v in table_renames.items(): to_run += rename_table(migrator, k, v)

just FK creation?

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.

2 participants