77377 2004-03-12 13:04:23 +0000 SQL code generator generates "circular reference" foreign keys 2006-11-20 22:36:56 +0000 1 1 2 Applications umbrello general 1.2 unspecified Linux RESOLVED FIXED NOR normal --- 0 matofesi umbrello-devel holster 0 oldest_to_newest 214635 0 matofesi 2004-03-12 13:04:24 +0000 Version: 1.2 (using KDE 3.2.0, Gentoo) Compiler: gcc version 3.3.2 20031218 (Gentoo Linux 3.3.2-r5, propolice-3.3-7) OS: Linux (i686) release 2.4.22-gentoo-r7 When you use Umbrello to design SQL scheme using agregations to simulate foreign keys constraints the code generators creates foreign key constarint in both tables instead of the "source" (role B - as I assume looking at the arrowhead of the aggregation connection - which is a bit misleading) table. Below is the simple patch - it makes code generator to check the objects of the aggregations and only create constraint in one table. In the other table generator also places the constraint, but only as an SQL comment. I'm not sure how well will it work, but at least it works for me. M. ----------------- umbrello-sqlwriter.patch --------------------------- --- kdesdk-3.2.0-orig/umbrello/umbrello/codegenerators/sqlwriter.cpp 2003-09-21 10:41:52.000000000 +0200 +++ kdesdk-3.2.0/umbrello/umbrello/codegenerators/sqlwriter.cpp 2004-03-12 12:15:23.000000000 +0100 @@ -89,8 +89,14 @@ UMLAssociationList aggregations = c->getAggregations(); if( forceSections() || !aggregations.isEmpty() ) { for(UMLAssociation* a = aggregations.first(); a; a = aggregations.next()) { - sql << ",\n\tCONSTRAINT " << a->getName() << " FOREIGN KEY (" << a->getMultiB() << - ") REFERENCES " << a->getObjectA()->getName() << "(" << a->getMultiA() << ")"; + if( a->getObjectA()->getID()==c->getID() ) { + sql << "\n-- CONSTRAINT " << a->getName() << " FOREIGN KEY (" << a->getMultiB() << + ") REFERENCES " << a->getObjectA()->getName() << "(" << a->getMultiA() << ")"; + } else { + sql << ",\n\tCONSTRAINT " << a->getName() << " FOREIGN KEY (" << a->getMultiB() << + ") REFERENCES " << a->getObjectA()->getName() << "(" << a->getMultiA() << ")"; + + } } } 215927 1 jr 2004-03-15 23:31:39 +0000 I've applied the patch, thanks for your contribution. 215929 2 jr 2004-03-15 23:32:50 +0000 *** Bug 76877 has been marked as a duplicate of this bug. *** 254189 3 seb.kde 2004-07-29 17:47:02 +0000 - foreign key constraint fixed - comma adding fixed for different scopes 254192 4 seb.kde 2004-07-29 17:50:08 +0000 Ah, I added the wrong comment a minute ago. This bug was already mostly fixed. I just removed the commented foreign key constraint, because it is nonsense and makes the code easier to read. 488185 5 18621 alexeyp 2006-11-20 08:29:16 +0000 Created attachment 18621 Patch to allow tables to have self-references Sometimes, you need something like this: create table t1 ( id int primary key, parent int references t1(id) ); The proposed patch allows it. Also, Umbrello sometimes tries to create (in the export file) the empty foreign keys with no table name and fields. These declarations come as a result of deleting associations and not accessible through the Umbrello interface. However, they are presented in the XMI file. This patch offers a primitive workaround for this problem, too, by ignoring such associations during export. 488187 6 alexeyp 2006-11-20 08:30:36 +0000 Forgot to tell: Umbrello version I'm reporting about is a part of KDE 3.5.5. 488353 7 okellogg 2006-11-20 20:29:26 +0000 In attachment id=18621, Alexey Parshin wrote: + if ( a->getRoleName(Uml::B) != "" && a->getRoleName(Uml::B) != "") { Hmm, twice the same expression? Please check. 488423 8 okellogg 2006-11-20 22:36:55 +0000 SVN commit 606555 by okellogg: Apply attachment 18621 by Alexey Parshin with slight modification. Thanks Alexey for contributing. CCBUG:77377 M +12 -8 sqlwriter.cpp --- branches/KDE/3.5/kdesdk/umbrello/umbrello/codegenerators/sqlwriter.cpp #606554:606555 @@ -86,14 +86,18 @@ UMLAssociationList aggregations = c->getAggregations(); if( forceSections() || !aggregations.isEmpty() ) { for(UMLAssociation* a = aggregations.first(); a; a = aggregations.next()) { - if( a->getObject(Uml::A)->getID() != c->getID() ) { - - sql << m_indentation << "," << m_endl << m_indentation - << "CONSTRAINT " << a->getName() << " FOREIGN KEY (" - << a->getRoleName(Uml::B) << ") REFERENCES " - << a->getObject(Uml::A)->getName() - << " (" << a->getRoleName(Uml::A) << ")"; - } + UMLObject *objA = a->getObject(Uml::A); + UMLObject *objB = a->getObject(Uml::B); + if (objA->getID() == c->getID() && objB->getID() != c->getID()) + continue; + QString roleNameA = a->getRoleName(Uml::A); + QString roleNameB = a->getRoleName(Uml::B); + if (roleNameA.isEmpty() || roleNameB.isEmpty()) + continue; + sql << m_indentation << "," << m_endl; + sql << m_indentation << "CONSTRAINT " << a->getName() + << " FOREIGN KEY (" << roleNameB << ") REFERENCES " + << objA->getName() << " (" << roleNameA << ")"; } } 18621 2006-11-20 08:29:16 +0000 2006-11-20 08:29:16 +0000 Patch to allow tables to have self-references 3.5.5-sql-export.patch text/plain 1505 alexeyp ZGlmZiAtdU5yIGtkZXNkay0zLjUuNS5vcmlnL3VtYnJlbGxvL3VtYnJlbGxvL2NvZGVnZW5lcmF0 b3JzL3NxbHdyaXRlci5jcHAga2Rlc2RrLTMuNS41L3VtYnJlbGxvL3VtYnJlbGxvL2NvZGVnZW5l cmF0b3JzL3NxbHdyaXRlci5jcHAKLS0tIGtkZXNkay0zLjUuNS5vcmlnL3VtYnJlbGxvL3VtYnJl bGxvL2NvZGVnZW5lcmF0b3JzL3NxbHdyaXRlci5jcHAJMjAwNi0wNy0yMiAxODoxMTo1Ny4wMDAw MDAwMDAgKzEwMDAKKysrIGtkZXNkay0zLjUuNS91bWJyZWxsby91bWJyZWxsby9jb2RlZ2VuZXJh dG9ycy9zcWx3cml0ZXIuY3BwCTIwMDYtMTEtMjAgMTY6NDI6MzUuMDAwMDAwMDAwICsxMTAwCkBA IC04NiwxNCArODYsMTUgQEAKICAgICBVTUxBc3NvY2lhdGlvbkxpc3QgYWdncmVnYXRpb25zID0g Yy0+Z2V0QWdncmVnYXRpb25zKCk7CiAgICAgaWYoIGZvcmNlU2VjdGlvbnMoKSB8fCAhYWdncmVn YXRpb25zLmlzRW1wdHkoKSApIHsKICAgICAgICAgZm9yKFVNTEFzc29jaWF0aW9uKiBhID0gYWdn cmVnYXRpb25zLmZpcnN0KCk7IGE7IGEgPSBhZ2dyZWdhdGlvbnMubmV4dCgpKSB7Ci0gICAgICAg ICAgICBpZiggYS0+Z2V0T2JqZWN0KFVtbDo6QSktPmdldElEKCkgIT0gYy0+Z2V0SUQoKSApIHsK LQotICAgICAgICAgICAgICAgIHNxbCA8PCBtX2luZGVudGF0aW9uIDw8ICIsIiA8PCBtX2VuZGwg PDwgbV9pbmRlbnRhdGlvbgotICAgICAgICAgICAgICAgIDw8ICJDT05TVFJBSU5UICIgPDwgYS0+ Z2V0TmFtZSgpIDw8ICIgRk9SRUlHTiBLRVkgKCIKLSAgICAgICAgICAgICAgICA8PCBhLT5nZXRS b2xlTmFtZShVbWw6OkIpIDw8ICIpIFJFRkVSRU5DRVMgIgotICAgICAgICAgICAgICAgIDw8IGEt PmdldE9iamVjdChVbWw6OkEpLT5nZXROYW1lKCkKLSAgICAgICAgICAgICAgICA8PCAiICgiIDw8 IGEtPmdldFJvbGVOYW1lKFVtbDo6QSkgPDwgIikiOwotICAgICAgICAgICAgfQorICAgICAgICAg ICAgaWYoIGEtPmdldE9iamVjdChVbWw6OkEpLT5nZXRJRCgpID09IGMtPmdldElEKCkgJiYgYS0+ Z2V0T2JqZWN0KFVtbDo6QiktPmdldElEKCkgIT0gYy0+Z2V0SUQoKSApCisJCWNvbnRpbnVlOwor CSAgICBpZiAoIGEtPmdldFJvbGVOYW1lKFVtbDo6QikgIT0gIiIgJiYgYS0+Z2V0Um9sZU5hbWUo VW1sOjpCKSAhPSAiIikgeworICAgICAgICAgICAgCSAgICBzcWwgPDwgbV9pbmRlbnRhdGlvbiA8 PCAiLCIgPDwgbV9lbmRsIDw8IG1faW5kZW50YXRpb24KKyAgICAgICAgICAgIAkgICAgPDwgIkNP TlNUUkFJTlQgIiA8PCBhLT5nZXROYW1lKCkgPDwgIiBGT1JFSUdOIEtFWSAoIgorICAgICAgICAg ICAgCSAgICA8PCBhLT5nZXRSb2xlTmFtZShVbWw6OkIpIDw8ICIpIFJFRkVSRU5DRVMgIgorICAg ICAgICAgICAgCSAgICA8PCBhLT5nZXRPYmplY3QoVW1sOjpBKS0+Z2V0TmFtZSgpCisgICAgICAg ICAgICAJICAgIDw8ICIgKCIgPDwgYS0+Z2V0Um9sZU5hbWUoVW1sOjpBKSA8PCAiKSI7CisJICAg IH0KICAgICAgICAgfQogICAgIH0KIAo=