split the references into 2 tables to allow using defered FKs in the future #75

Merged
merged 1 commit into from Jun 7, 2013

Conversation

Projects
None yet
2 participants
@lsmith77
Member

lsmith77 commented Nov 14, 2012

Upgrade steps:
Upgrade jackalope/jackalope-doctrine-dbal
Run app/console doctrine:phpcr:init:dbal --dump-sql
Copy and execute all tables, indexes etc related to phpcr_nodes_references and phpcr_nodes_weakreferences.

Run the following SQL statements:
INSERT INTO phpcr_nodes_references ( source_id, source_property_name, target_id )
SELECT source_id, source_property_name, target_id
FROM phpcr_nodes_foreignkeys WHERE type = 9;
INSERT INTO phpcr_nodes_weakreferences ( source_id, source_property_name, target_id )
SELECT source_id, source_property_name, target_id
FROM phpcr_nodes_foreignkeys WHERE type = 10;

Finally clean up the old table:
DROP TABLE phpcr_nodes_foreignkeys;

In the future we can now implement the deferred FK support:
Doctrine DBAL supports this for PostgreSQL, it should be possible to add this to SQLite as well:
doctrine/dbal#220

http://www.postgresql.org/docs/9.2/static/sql-set-constraints.html
http://www.sqlite.org/foreignkeys.html#fk_deferred

@dbu

This comment has been minimized.

Show comment
Hide comment
@dbu

dbu Feb 12, 2013

Member

is this still relevant and needed? it seems to be in conflict now - i am pretty sure #87 does some incompatible changes.

Member

dbu commented Feb 12, 2013

is this still relevant and needed? it seems to be in conflict now - i am pretty sure #87 does some incompatible changes.

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 Feb 13, 2013

Member

yes it's still relevant. but it's not super duper must have and its a fair bit of work

Member

lsmith77 commented Feb 13, 2013

yes it's still relevant. but it's not super duper must have and its a fair bit of work

@dbu

This comment has been minimized.

Show comment
Hide comment
@dbu

dbu Apr 18, 2013

Member

lets for now just split the tables but still do the integrity checks in php - we will need that for mysql anyways.

Member

dbu commented Apr 18, 2013

lets for now just split the tables but still do the integrity checks in php - we will need that for mysql anyways.

@ghost ghost assigned lsmith77 May 13, 2013

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 May 28, 2013

Member

this PR is now ready for final review. we also then need to document the migration steps. essentially what people need to do is create the two new tables and then run insert into .. select statements to migrate the data and finally remove the old table.

Member

lsmith77 commented May 28, 2013

this PR is now ready for final review. we also then need to document the migration steps. essentially what people need to do is create the two new tables and then run insert into .. select statements to migrate the data and finally remove the old table.

- $nodeIds = array_keys($this->referencesToDelete);
- $params = array(PropertyType::REFERENCE, $nodeIds, $nodeIds);
+ $params = array(array_keys($this->referencesToDelete));
+

This comment has been minimized.

@dbu

dbu May 29, 2013

Member

i think a few comments would help a lot here to understand the code. if i get it right, we first delete all references originating from nodes we deleted, as we can always delete them. then we check if there are references pointing to the node being deleted, which after we deleted source references from this changeset means there still are references from nodes that are not deleted in the same save.

what about deleting the weak references too?

@dbu

dbu May 29, 2013

Member

i think a few comments would help a lot here to understand the code. if i get it right, we first delete all references originating from nodes we deleted, as we can always delete them. then we check if there are references pointing to the node being deleted, which after we deleted source references from this changeset means there still are references from nodes that are not deleted in the same save.

what about deleting the weak references too?

@dbu

This comment has been minimized.

Show comment
Hide comment
@dbu

dbu May 29, 2013

Member

looks good to me. if you want to explain the steps where i commented, please do but if you think its not needed its ok.

for the update help i guess we should start CHANGELOG.md in this repository too.

regarding recent doctrine dbal, in the cmf context that is a non-issue as phpcr-odm requires doctrine commons 2.4 and only the very latest dbal 2.3.x version declares itself compatible with that... i would be in favor of upping our requriements there in order to get the foreign keys.

Member

dbu commented May 29, 2013

looks good to me. if you want to explain the steps where i commented, please do but if you think its not needed its ok.

for the update help i guess we should start CHANGELOG.md in this repository too.

regarding recent doctrine dbal, in the cmf context that is a non-issue as phpcr-odm requires doctrine commons 2.4 and only the very latest dbal 2.3.x version declares itself compatible with that... i would be in favor of upping our requriements there in order to get the foreign keys.

@lsmith77

This comment has been minimized.

Show comment
Hide comment
@lsmith77

lsmith77 May 29, 2013

Member

regarding recent doctrine dbal, in the cmf context that is a non-issue as phpcr-odm requires doctrine commons 2.4 and only the very latest dbal 2.3.x version declares itself compatible with that... i would be in favor of upping our requriements there in order to get the foreign keys.

we can do it once we implement support for using deferred FK support

Member

lsmith77 commented May 29, 2013

regarding recent doctrine dbal, in the cmf context that is a non-issue as phpcr-odm requires doctrine commons 2.4 and only the very latest dbal 2.3.x version declares itself compatible with that... i would be in favor of upping our requriements there in order to get the foreign keys.

we can do it once we implement support for using deferred FK support

lsmith77 added a commit that referenced this pull request Jun 7, 2013

Merge pull request #75 from jackalope/split_reference_tables
split the references into 2 tables to allow using defered FKs in the future

@lsmith77 lsmith77 merged commit cd8851c into master Jun 7, 2013

@lsmith77 lsmith77 deleted the split_reference_tables branch Jun 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment