PEDSnet / pedsnetdcc

CLI tool for PEDSnet data coordinating center ETL tasks
Other
0 stars 0 forks source link

Add move_primary_keys function #46

Closed murphyke closed 8 years ago

murphyke commented 8 years ago

The only change from the original hand-generated SQL is the addition of CASCADE to the drop statement, which will have the effect of automatically dropping the index underlying the primary key constraint and also of dropping any foreign keys pointing to the primary key that might still exist in the original table. This was due to laziness on my part and seems benign to me, since ordinarily those things would have been dropped already. Thoughts?

murphyke commented 8 years ago

My other comment is that this code would benefit performance-wise from nestable StatementSet and StatementList, but we might be able to live without this feature for a bit. We could have an abstract StatementExecuter class that would be implemented by Statement, StatementSet, and StatementList. Among other things, this would make it microscopically easier for StatementList and StatementSet to validate objects added to them.

gracebrownecodes commented 8 years ago

Given the multi-schema transformation plan laid out in #30, this should become an add_primary_keys function, since the transformed tables will be in a new schema and therefore the primary key names will not collide.

murphyke commented 8 years ago

OK, the switch to add_primary_keys is almost done.

murphyke commented 8 years ago

Changed move to add. There is a new utils.conn_str_with_search_path function that will be handy for dealing with multiple schemas.

gracebrownecodes commented 8 years ago

Done reviewing.

murphyke commented 8 years ago

Thanks, will make the changes.

gracebrownecodes commented 8 years ago

LGTM. Merging.