Jump to:

1971 Posts in 1275 Topics by 607 members

Form Questions

SilverStripe Forums » Form Questions » Possible patch for field collisions

Moderators: martimiz, Sean, biapar, Willr, Ingo, swaiba, simon_w

Page: 1
Go to End
Author Topic: 708 Views
  • nykayfabe
    Avatar
    Community Member
    4 Posts

    Possible patch for field collisions Link to this post

    Please forgive me if this is all wrong. I'm working from an older version of SS (2.3.1) which was heavily customized and hopelessly out of sync, making an upgrade a major hassle. Anyway, when a page type is changed and a field of the same name exists in both the old and new type, sometimes the field value is not retrieved correctly (the value from the old page type record comes through). I think the fix I came up with would work even with the most current build but it's so simple that I'm doubting it. In sapphire/core/model/DataObject.php > buildSQL change:

    $query->from[$tableClass] = "LEFT JOIN \"$tableClass\" ON \"$tableClass\".\"ID\" = \"$baseClass\".\"ID\"";

    to:

    $query->from[$tableClass] = "LEFT JOIN `$tableClass` ON `$tableClass`.ID = `$baseClass`.ID AND '$tableClass' = `$baseClass`.ClassName";

    The latest version of DataObject attempts to fix this by using the array, collidingFields, and then adding a WHEN clause to the query. I actually attempted to flesh this out before I came up with the above and it resulted in some humdongous queries which crashed our live site.

    That said, does anyone see anything wrong with the fix I've posted? FWIW, it seems to be working fine on my test machine in that all pages are rendered correctly and the fields which were previously getting the wrong value in the CMS are now correct. (Incidentally, the collision problem can also be resolved by manually deleting the record from the old page type table, but that's a pain to do on the regular.) Thanks in advance.

  • Willr
    Avatar
    Forum Moderator
    5462 Posts

    Re: Possible patch for field collisions Link to this post

    That said, does anyone see anything wrong with the fix I've posted?

    Well the first issue is that your patch isn't using the correct format for escaping queries so it wouldn't be considered for merging back. See http://doc.silverstripe.org/coding-conventions#writing_queries_for_database_abstraction. As for the functionality of the patch I'm not qualified to know it'll break anything - have you run the unit tests after making that change?.

  • ajshort
    Avatar
    Community Member
    244 Posts

    Re: Possible patch for field collisions Link to this post

    OK, from what I understand, if you have a tree of classes that runs SiteTree -> Page -> BlogEntry, and you perform a DataObject::get_one('SiteTree'), the following joins will be created:

    + SiteTree
       + Page -> LEFT JOIN "Page" ON "Page"."ID" = "SiteTree"."ID" AND "SiteTree"."ClassName" = 'Page'
          + BlogPage -> LEFT JOIN "BlogPage" ON "BlogPage"."ID" = "SiteTree"."ID" AND "SiteTree"."ClassName" = 'BlogPage'

    If you have a record with ClassName "BlogEntry" across all those tables, you would expect all the data from all the tables to be returned in that record. However, since "SiteTree"."ClassName" == 'BlogPage', the join condition for the Page class will return false (since it is comparing it wirh 'Page') - so any data from the Page table won't be returned.

    I could be wrong, but I think this patch will break things.

  • nykayfabe
    Avatar
    Community Member
    4 Posts

    Re: Possible patch for field collisions Link to this post

    Thanks for your reply aj. I think I understand what you're saying but I don't believe it's a problem. I ran DataObject::get_one('SiteTree') in the controller from a page with the same hierarchy as your example, SiteTree -> Page -> IndividualEntry, and it produces a query that includes this:

    LEFT JOIN `Article` ON `Article`.ID = `SiteTree`.ID AND 'Article' = `SiteTree`.ClassName
    LEFT JOIN `CareerOpportunities` ON `CareerOpportunities`.ID = `SiteTree`.ID AND 'CareerOpportunities' = `SiteTree`.ClassName

    ...so on and so forth. The query, in fact, does return every record in SiteTree because the second AND clause in the JOIN is always satisfied because there is no WHERE clause to limit the ClassName. In the CMS, however, the queries have a WHERE clause (e.g.: "WHERE `SiteTree`.`ID` = 127") which means that the SiteTree.ClassName being evaluated is the one from the record with that specific ID, the ID of the page you're editing.

    A different query fired from the same page's controller, DataObject::get_one('Article') correctly only returns all records in the Article table:
    SELECT `SiteTree`.*, `Article`.*, `SiteTree`.ID, if(`SiteTree`.ClassName,`SiteTree`.ClassName,'SiteTree') AS RecordClassName FROM `SiteTree` LEFT JOIN `Article` ON `Article`.ID = `SiteTree`.ID AND 'Article' = `SiteTree`.ClassName WHERE (`SiteTree`.ClassName IN ('Article')) ORDER BY Sort

    If I've misunderstood your concerns or misinterpreted your example, please let me know. Thanks again.

  • ajshort
    Avatar
    Community Member
    244 Posts

    Re: Possible patch for field collisions Link to this post

    I just tried your patch on my local site and it broke a heap of stuff. If i have the page structure SiteTree -> Page -> CustomPage, where both Page and CustomPage have custom db fields, and try something like:

    var_dump(DataObject::get_by_id('Page', 1)->Test);

    it will return null, whereas when i revert the patch it works fine.

  • nykayfabe
    Avatar
    Community Member
    4 Posts

    Re: Possible patch for field collisions Link to this post

    I guess it only works with our hacked up version of 2.3.1 because if I do the following, it correctly prints the title of the first page in SiteTree:
    var_dump(DataObject::get_by_id('Page', 1)->Title);

    Weird, wild stuff. Thanks for your help and I'm sorry that my "solution" isn't universally applicable.

    708 Views
Page: 1
Go to Top

Want to know more about the company that brought you SilverStripe? Then check out SilverStripe.com

Comments on this website? Please give feedback.