emmett-framework / emmett

The web framework for inventors
BSD 3-Clause "New" or "Revised" License
1.06k stars 71 forks source link

Problem with firebird and migrations #191

Closed josejachuf closed 7 years ago

josejachuf commented 7 years ago

Hello @gi0baro

In pydal 17.3 it still has the same problem. I had sent a pull request that solved it, but the code stays the same. I'm going to report it directly to pydal. But that's not what I want to show now.

Fixing pydal/adapters/firebird.py locally and using the bloggy example, the following happens:

Reviewing the sql created in orm/migratios/engine.py we have:

Note: In firebird if the table or field names are enclosed in quotation marks, it is created exactly as given; If not, it is created in uppercase

CREATE TABLE "users"(
    "id" INTEGER PRIMARY KEY,
    "created_at" TIMESTAMP,
    "updated_at" TIMESTAMP,
    "email" VARCHAR(255),
    "password" VARCHAR(512),
    "registration_key" VARCHAR(512) DEFAULT '',
    "reset_password_key" VARCHAR(512) DEFAULT '',
    "registration_id" VARCHAR(512) DEFAULT '',
    "first_name" VARCHAR(128) NOT NULL,
    "last_name" VARCHAR(128) NOT NULL
);

Creating by hand from firebird administrator: OK

CREATE TABLE "auth_groups"(
    "id" INTEGER PRIMARY KEY,
    "created_at" TIMESTAMP,
    "updated_at" TIMESTAMP,
    "role" VARCHAR(255) DEFAULT '',
    "description" BLOB SUB_TYPE 1
);

Creating by hand from firebird administrator: OK

CREATE TABLE "auth_memberships"(
    "id" INTEGER PRIMARY KEY,
    "created_at" TIMESTAMP,
    "updated_at" TIMESTAMP,
    "user" INTEGER REFERENCES users (id) ON DELETE CASCADE,
    "auth_group" INTEGER REFERENCES auth_groups (id) ON DELETE CASCADE
);

Creating by hand from firebird administrator: Fail. The fault is because the name of the table and the field are not enclosed in quotation marks. If corrected as follows works fine

CREATE TABLE "auth_memberships"(
    "id" INTEGER PRIMARY KEY,
    "created_at" TIMESTAMP,
    "updated_at" TIMESTAMP,
    "user" INTEGER REFERENCES "users" ("id") ON DELETE CASCADE,
    "auth_group" INTEGER REFERENCES "auth_groups" ("id") ON DELETE CASCADE
);

Jose

gi0baro commented 7 years ago

@josejachuf will inspect the migration code trying to solve this in the next few days.

josejachuf commented 7 years ago

@gi0baro I already solved in my environment the names of the tables and fields in quotation marks.

def _gen_reference(self, tablename, column, tfks):
    ....
    else:
            csql_info = dict(
                index_name=self.dialect.quote(column.name + '__idx'),
                field_name=rfieldname,
                constraint_name=self.dialect.quote(constraint_name),
                foreign_key='"%s" ("%s")' % (rtablename, rfieldname),
                on_delete_action=column.ondelete)
            csql_info['null'] = ' NOT NULL' if column.notnull else \
                self.dialect.allow_null
    ....

The tables are now created, but adapter.create_sequence_and_triggers(query, table) is not called. This does not create the sequences and triggers, needed in firebird to be assigned to the id values.

gi0baro commented 7 years ago

@josejachuf ok, will check that too. This will take a few days since I have to re-test it with several dbms.

josejachuf commented 7 years ago

Thanks @gi0baro. If you create a firebird database it should be with page size = 8192

gi0baro commented 7 years ago

Should be fixed with d89c87b

josejachuf commented 7 years ago

@gi0baro, Two notes about this:

1) In orm/adapters._create_table_firebird, the table name must be enclosed in quotation marks

'create trigger %s for "%s" active before insert position 0 as\n'

def _create_table_firebird(dialect, tablename, fields):
    rv = _create_table(dialect, tablename, fields)
    sequence_name = dialect.sequence_name(tablename)
    trigger_name = dialect.trigger_name(tablename)
    trigger_sql = (
        'create trigger %s for "%s" active before insert position 0 as\n'
        'begin\n'
        'if(new."id" is null) then\n'
        'begin\n'
        'new."id" = gen_id(%s, 1);\n'
        'end\n'
        'end;')
    rv.extend([
        'create generator %s;' % sequence_name,
        'set generator %s to 0;' % sequence_name,
        trigger_sql % (trigger_name, tablename, sequence_name)
    ])
    return rv

With this modification works fine

2) The weppy_scheme table must be created as a firebird table, this is to create the sequence and associated trigger

gi0baro commented 7 years ago

@josejachuf should be fixed with 470cc2a

josejachuf commented 7 years ago

@gi0baro Work fine.