perseas / Pyrseas

Provides utilities for Postgres database schema versioning.
https://perseas.github.io/
BSD 3-Clause "New" or "Revised" License
395 stars 67 forks source link

yamltodb doesn't generate migration on index changes #144

Closed darthunix closed 8 years ago

darthunix commented 8 years ago

If I modify index's keys in yaml schema, yamltodb doesn't generate any migrations. For example I have table test

 table test:
    columns:
    - value:
        type: text
    indexes:
      test_idx:
        keys:
        - value
    owner: postgres

I modify table test:

  table test:
    columns:
    - value:
        type: text
    indexes:
      test_idx:
        keys:
        - btrim(value)
    owner: postgres

I expect yamltodb create migration like

drop index test_idx;
create index test_idx on test(trim(value));

but right now on branch r0.7 yamltodb generates empty output... Where is the problem?

testdb_orig_yaml.txt testdb_modified_yaml.txt

jmafc commented 8 years ago

Yes, I'm afraid that is one of the TODOs: see pyrseas/dbobject/index.py line 130. This is similar to issue #115 (primary key changes--which did get addressed late in 2015) and I believe some still open TODOs in constraint.py.

darthunix commented 8 years ago

Ok, I'll try to fix this problem on a weekend. For constraint.py I'll open separate issues))

darthunix commented 8 years ago

I have fixed the problem with indexes in my local branch. I think this fix is too small for a pull request. So I'll try to fix all TODOs in constraint.py and make a big pull request on a week.

jmafc commented 8 years ago

With this and the other changes, I'm thinking we ought to do a release, either 0.7.3 or 0.8, so that it's easier for people to install from PyPI, etc.

darthunix commented 8 years ago

Finally I have done a pull request, closing this task. Before making a minor release we should close one more bug. Yesterday dbtoyaml crashed on dumping a foreign table. I thing it was some parsing error. I'll try to find out how to reproduce this bug and open an issue.

jmafc commented 8 years ago

Although I merged your PR, now I see there are problems merging your changes into master. The trouble is that in master there is already a partial solution (as I mentioned above, re issue #115). For example, PR #126 added five new tests to tests/dbobject/test_constraint.py and at least some of them are now failing in master. I have to check whether some of those are redundant with the ones you added.

This is just a heads up. You don't need to take any action. I'll handle it, because I want to try to merge them also into deptrack (having three active branches tends to create these sort of situations). In part the issue is not having rules about where a particular issue should be fixed.

darthunix commented 8 years ago

The main problem is that I fix the problems I face at work using pyrseas. Right now I am using r0.7 because master branch still has an opened issue #139 that breaks yamltodb. After closing #139 I promise to switch to master and do not touch r0.7)) Sorry for making problems with my PR...

jmafc commented 8 years ago

No need to apologize. If we release 0.7.3 (and I'm leaning towards that because I'd rather have 0.8 as the deptrack release), then it's better to have the changes in r0.7 and then cross them to master, rather than the other way around.