Moodle
  1. Moodle
  2. MDL-26504

External blog posts not removed from {post} table when the blog is unregistered

    Details

    • Type: Bug Bug
    • Status: Closed
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 2.0.1, 2.2.4, 2.3.1, 2.4
    • Fix Version/s: 2.2.5, 2.3.2
    • Component/s: Blog
    • Labels:
    • Database:
      Any
    • Testing Instructions:
      Hide

      For this test you'll need to be comfortable with SQL. Test across all the supported DB drivers.

      Log in as admin.
      Go to the external blogs screen (my profile settings > blogs > External blogs)
      Add two blogs. Each region covered by the BBC has its own RSS feed if you need some to test with. http://www.bbc.co.uk/news/world/asia/
      Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content.
      Through the external blog admin page delete one of the external blogs.
      Check that its posts are gone but that the posts from the other blog are still there.

      Show
      For this test you'll need to be comfortable with SQL. Test across all the supported DB drivers. Log in as admin. Go to the external blogs screen (my profile settings > blogs > External blogs) Add two blogs. Each region covered by the BBC has its own RSS feed if you need some to test with. http://www.bbc.co.uk/news/world/asia/ Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content. Through the external blog admin page delete one of the external blogs. Check that its posts are gone but that the posts from the other blog are still there.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_20_STABLE, MOODLE_22_STABLE, MOODLE_23_STABLE, MOODLE_24_STABLE
    • Fixed Branches:
      MOODLE_22_STABLE, MOODLE_23_STABLE
    • Pull Master Branch:
      MDL-26504_blog_unregister
    • Rank:
      16100

      Description

      Because the posts are not removed on the blog un-registartion, they still appear at the user site blog page. They should be removed immediately after the user unregisters the blog.

        Activity

        David Mudrak created issue -
        David Mudrak made changes -
        Field Original Value New Value
        Labels triaged
        Assignee moodle.com [ moodle.com ] Andrew Davis [ andyjdavis ]
        Fix Version/s STABLE backlog [ 10463 ]
        Hide
        David Mudrak added a comment -

        Triaged as suggested by Eloy. Thanks Andrew.

        Show
        David Mudrak added a comment - Triaged as suggested by Eloy. Thanks Andrew.
        Martin Dougiamas made changes -
        Workflow MDL Workflow [ 68085 ] MDL Full Workflow [ 75941 ]
        Hide
        Sara Hoepner added a comment - - edited

        I would also suggest that the person who registered the external blog, be able to delete individual posts from the registered blog as well.

        Update--I see this has been reported in MDL-27427

        Show
        Sara Hoepner added a comment - - edited I would also suggest that the person who registered the external blog, be able to delete individual posts from the registered blog as well. Update--I see this has been reported in MDL-27427
        Hide
        Paul Haddad added a comment -

        This is a huge problem, I registered a blog thread and I cannot as full admin delete these posts. This is ridiculous, and now I had to change the setting to hide blogs from each user because they flooded the student blogs area. Please fix this issue, for 2.0 and up.

        Show
        Paul Haddad added a comment - This is a huge problem, I registered a blog thread and I cannot as full admin delete these posts. This is ridiculous, and now I had to change the setting to hide blogs from each user because they flooded the student blogs area. Please fix this issue, for 2.0 and up.
        Andrew Davis made changes -
        Fix Version/s STABLE Sprint 23 [ 12358 ]
        Fix Version/s STABLE backlog [ 10463 ]
        Andrew Davis made changes -
        Status Open [ 1 ] Development in progress [ 3 ]
        Hide
        Paul Haddad added a comment -

        I had to remove my account and create a new one as a quick fix to remove all the blog posts that were associated with my account. This is the best we can do on the front end in the meantime.

        Show
        Paul Haddad added a comment - I had to remove my account and create a new one as a quick fix to remove all the blog posts that were associated with my account. This is the best we can do on the front end in the meantime.
        Andrew Davis made changes -
        Peer reviewer phalacee
        Database Any [ 10000 ]
        Difficulty Easy [ 10023 ]
        Hide
        Andrew Davis added a comment -

        Adding a branch with a solution. I'll create the other branches after peer review. This took a bit longer than I expected. We're storing a row ID in a text column.

        Show
        Andrew Davis added a comment - Adding a branch with a solution. I'll create the other branches after peer review. This took a bit longer than I expected. We're storing a row ID in a text column.
        Andrew Davis made changes -
        Andrew Davis made changes -
        Testing Instructions For this test you'll need to be comfortable with SQL.

        Log in as admin.
        Go to the external blogs screen (my profile settings > blogs > External blogs)
        Add two blogs. Each region covered by the BBC has one if you need some to test with. http://www.bbc.co.uk/news/world/asia/
        Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content
        Through the external blog admin page delete one of the external blogs.
        Check that its posts are gone but that the posts from the other blog are still there.
        Andrew Davis made changes -
        Testing Instructions For this test you'll need to be comfortable with SQL.

        Log in as admin.
        Go to the external blogs screen (my profile settings > blogs > External blogs)
        Add two blogs. Each region covered by the BBC has one if you need some to test with. http://www.bbc.co.uk/news/world/asia/
        Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content
        Through the external blog admin page delete one of the external blogs.
        Check that its posts are gone but that the posts from the other blog are still there.
        For this test you'll need to be comfortable with SQL.

        Log in as admin.
        Go to the external blogs screen (my profile settings > blogs > External blogs)
        Add two blogs. Each region covered by the BBC has its own RSS feed if you need some to test with. http://www.bbc.co.uk/news/world/asia/
        Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content.
        Through the external blog admin page delete one of the external blogs.
        Check that its posts are gone but that the posts from the other blog are still there.
        Andrew Davis made changes -
        Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
        Jason Fowler made changes -
        Original Estimate 0 minutes [ 0 ]
        Remaining Estimate 0 minutes [ 0 ]
        Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
        Hide
        Jason Fowler added a comment -

        Looks good to me, the usage of sql_compare_text() looks messy, but I can't see a way around the problem

        Show
        Jason Fowler added a comment - Looks good to me, the usage of sql_compare_text() looks messy, but I can't see a way around the problem
        Jason Fowler made changes -
        Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
        Hide
        Andrew Davis added a comment -

        Adding branches for 2.3 and 2.2 and putting this up for integration.

        Show
        Andrew Davis added a comment - Adding branches for 2.3 and 2.2 and putting this up for integration.
        Andrew Davis made changes -
        Andrew Davis made changes -
        Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
        Dan Poltawski made changes -
        Currently in integration Yes [ 10041 ]
        Dan Poltawski made changes -
        Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
        Integrator poltawski
        Hide
        Dan Poltawski added a comment -

        Wow, this is completely screwy isn't it!

        1. Understandably (as its confusing) the use of sql_compare_text in the second case isn't quite right. Instead you should do $DB->sql_compare_text('content') . ' = ' . $DB->sql_compare_text('?') and then pass $delete as a parameter along with userid and module. The way that its written at the moment will put the parameter directly in the SQL rather than using parameters correctly.
        2. I notice that other places looking for external blogs check for 'uniqueid' being set. Should a $DB->sql_isnotempty() be put on uniqueid to ensure this condition is met to be consistent?

        Reopening for the first point, as it represents and sql injection risk.

        Show
        Dan Poltawski added a comment - Wow, this is completely screwy isn't it! Understandably (as its confusing) the use of sql_compare_text in the second case isn't quite right. Instead you should do $DB->sql_compare_text('content') . ' = ' . $DB->sql_compare_text('?') and then pass $delete as a parameter along with userid and module. The way that its written at the moment will put the parameter directly in the SQL rather than using parameters correctly. I notice that other places looking for external blogs check for 'uniqueid' being set. Should a $DB->sql_isnotempty() be put on uniqueid to ensure this condition is met to be consistent? Reopening for the first point, as it represents and sql injection risk.
        Dan Poltawski made changes -
        Status Integration review in progress [ 10004 ] Reopened [ 4 ]
        Andrew Davis made changes -
        Hide
        Andrew Davis added a comment -

        I believe I've fixed both of those issues. I have added the not empty check for uniquehash. Its an odd field and Im not 100% sure of its role however we're better off incorrectly leaving old blog posts in the database rather than accidentally deleting something we should keep.

        Show
        Andrew Davis added a comment - I believe I've fixed both of those issues. I have added the not empty check for uniquehash. Its an odd field and Im not 100% sure of its role however we're better off incorrectly leaving old blog posts in the database rather than accidentally deleting something we should keep.
        Andrew Davis made changes -
        Status Reopened [ 4 ] Waiting for peer review [ 10012 ]
        Hide
        Andrew Davis added a comment -

        I've updated the master version. I'll be away for a few hours. If this gets integrated in the mean time it should be able to be cherry picked cleanly.

        Show
        Andrew Davis added a comment - I've updated the master version. I'll be away for a few hours. If this gets integrated in the mean time it should be able to be cherry picked cleanly.
        Eloy Lafuente (stronk7) made changes -
        Currently in integration Yes [ 10041 ]
        Jason Fowler made changes -
        Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
        Hide
        Jason Fowler added a comment -

        the

        ' AND '.

        line could use cleaning up, moving the first ' to the end of the line above, other than that, it seems to fix the issues Dan raised

        Show
        Jason Fowler added a comment - the ' AND '. line could use cleaning up, moving the first ' to the end of the line above, other than that, it seems to fix the issues Dan raised
        Jason Fowler made changes -
        Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
        Hide
        Andrew Davis added a comment -

        Submitting for integration.

        Dear integrators, I'm having some git trouble so couldn't create a 2.3 and 2.2 version. It should however be cherry-pickable.

        Show
        Andrew Davis added a comment - Submitting for integration. Dear integrators, I'm having some git trouble so couldn't create a 2.3 and 2.2 version. It should however be cherry-pickable.
        Andrew Davis made changes -
        Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Aparup Banerjee made changes -
        Currently in integration Yes [ 10041 ]
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Oh my, using text for storing those ids.. We really need to move the posts table to use the component, area, itemid behavior ASAP.

        The main question here is what happens with already orphaned posts, shouldn't we be deleting them (aka, upgrade step?). Else the bad behavior will continue happening, isn't it?

        I don't think this can land without them getting fixed (deleted). So reopening, sorry.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Oh my, using text for storing those ids.. We really need to move the posts table to use the component, area, itemid behavior ASAP. The main question here is what happens with already orphaned posts , shouldn't we be deleting them (aka, upgrade step?). Else the bad behavior will continue happening, isn't it? I don't think this can land without them getting fixed (deleted). So reopening, sorry. Ciao
        Eloy Lafuente (stronk7) made changes -
        Status Waiting for integration review [ 10010 ] Reopened [ 4 ]
        Hide
        Andrew Davis added a comment -

        I've added a second commit containing upgrade code to clear out the orphan external blog posts. Putting this up for peer review.

        Show
        Andrew Davis added a comment - I've added a second commit containing upgrade code to clear out the orphan external blog posts. Putting this up for peer review.
        Andrew Davis made changes -
        Status Reopened [ 4 ] Waiting for peer review [ 10012 ]
        Eloy Lafuente (stronk7) made changes -
        Currently in integration Yes [ 10041 ]
        Andrew Davis made changes -
        Status Waiting for peer review [ 10012 ] Development in progress [ 3 ]
        Hide
        Andrew Davis added a comment -

        After a chat with Eloy Ive removed the NOT IN due to concerns about performance on large datasets. However, for the life of me, I cannot get this query to work. Not sure what Im doing wrong.

        Show
        Andrew Davis added a comment - After a chat with Eloy Ive removed the NOT IN due to concerns about performance on large datasets. However, for the life of me, I cannot get this query to work. Not sure what Im doing wrong.
        Hide
        David Mudrak added a comment -

        Hi Andrew. Sorry, I did not dive too deep into this - but this caught my eyes:

        WHERE b.id = p.content
        

        at least, it seems like integers are compared with a text field here, no? Can it be the reason? And what error do you get?

        Show
        David Mudrak added a comment - Hi Andrew. Sorry, I did not dive too deep into this - but this caught my eyes: WHERE b.id = p.content at least, it seems like integers are compared with a text field here, no? Can it be the reason? And what error do you get?
        Hide
        Rajesh Taneja added a comment -

        Hello David,

        I thought the same, but seems it is correct. For external blogs we keep external_blog.id in post.content.

        @Andrew: It seems like a problem with post alias. Can you try p.uniquehash <> '' instead of "$DB->sql_isnotempty('post', 'uniquehash', false, false);"

        Show
        Rajesh Taneja added a comment - Hello David, I thought the same, but seems it is correct. For external blogs we keep external_blog.id in post.content. @Andrew: It seems like a problem with post alias. Can you try p.uniquehash <> '' instead of "$DB->sql_isnotempty('post', 'uniquehash', false, false);"
        Hide
        Andrew Davis added a comment - - edited

        Still no luck. The error Im getting is below.

        Debug info: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'p
        WHERE mdl_post.module = 'blog_external'
        ' at line 1
        DELETE FROM mdl_post p
        WHERE mdl_post.module = ?
        AND NOT EXISTS (SELECT b.id FROM mdl_blog_external b where b.id = p.content)
        AND p <> ""
        [array (
        0 => 'blog_external',
        )]
        Error code: dmlwriteexception
        Stack trace:
        
            line 410 of /lib/dml/moodle_database.php: dml_write_exception thrown
            line 860 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end()
            line 1129 of /lib/db/upgrade.php: call to mysqli_native_moodle_database->execute()
            line 1483 of /lib/upgradelib.php: call to xmldb_main_upgrade()
            line 271 of /admin/index.php: call to upgrade_core()
        

        Update: I've now got a working query. Its a bit odd but aliasing the post table seemed to break it for reasons I don't quite understand. If anyone has a better working alternative by all means post it. Please try it before posting as whatever idea first pop into your head I've probably already tried it during what has been a rather frustrating morning.

        Show
        Andrew Davis added a comment - - edited Still no luck. The error Im getting is below. Debug info: You have an error in your SQL syntax; check the manual that corresponds to your MySQL server version for the right syntax to use near 'p WHERE mdl_post.module = 'blog_external' ' at line 1 DELETE FROM mdl_post p WHERE mdl_post.module = ? AND NOT EXISTS (SELECT b.id FROM mdl_blog_external b where b.id = p.content) AND p <> "" [array ( 0 => 'blog_external', )] Error code: dmlwriteexception Stack trace: line 410 of /lib/dml/moodle_database.php: dml_write_exception thrown line 860 of /lib/dml/mysqli_native_moodle_database.php: call to moodle_database->query_end() line 1129 of /lib/db/upgrade.php: call to mysqli_native_moodle_database->execute() line 1483 of /lib/upgradelib.php: call to xmldb_main_upgrade() line 271 of /admin/index.php: call to upgrade_core() Update: I've now got a working query. Its a bit odd but aliasing the post table seemed to break it for reasons I don't quite understand. If anyone has a better working alternative by all means post it. Please try it before posting as whatever idea first pop into your head I've probably already tried it during what has been a rather frustrating morning.
        Andrew Davis made changes -
        Status Development in progress [ 3 ] Waiting for peer review [ 10012 ]
        Jason Fowler made changes -
        Status Waiting for peer review [ 10012 ] Peer review in progress [ 10013 ]
        Hide
        Jason Fowler added a comment -

        Looks fine to me Andrew

        Show
        Jason Fowler added a comment - Looks fine to me Andrew
        Jason Fowler made changes -
        Status Peer review in progress [ 10013 ] Development in progress [ 3 ]
        Andrew Davis made changes -
        Status Development in progress [ 3 ] Waiting for integration review [ 10010 ]
        Hide
        Eloy Lafuente (stronk7) added a comment -

        The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

        TIA and ciao

        Show
        Eloy Lafuente (stronk7) added a comment - The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week. TIA and ciao
        Sam Hemelryk made changes -
        Currently in integration Yes [ 10041 ]
        Eloy Lafuente (stronk7) made changes -
        Integrator poltawski
        Aparup Banerjee made changes -
        Status Waiting for integration review [ 10010 ] Integration review in progress [ 10004 ]
        Integrator nebgor
        Hide
        Aparup Banerjee added a comment -

        thanks for the many reviews and iterations here.

        i've cherry-picked these 2 commits to 22 and 23. its in integration master too.

        Andrew: perhaps the sql issue you're encountering could be a MDL issue to help improve the dml API.

        Show
        Aparup Banerjee added a comment - thanks for the many reviews and iterations here. i've cherry-picked these 2 commits to 22 and 23. its in integration master too. Andrew: perhaps the sql issue you're encountering could be a MDL issue to help improve the dml API.
        Aparup Banerjee made changes -
        Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
        Affects Version/s 2.3.1 [ 12253 ]
        Affects Version/s 2.2.4 [ 12162 ]
        Affects Version/s 2.4 [ 12255 ]
        Fix Version/s 2.2.5 [ 12352 ]
        Fix Version/s 2.3.2 [ 12353 ]
        Hide
        Aparup Banerjee added a comment -

        the upgrade bit is giving me this under postgres:

        Debug info: ERROR: operator does not exist: bigint = text
        LINE 3: ... (SELECT b.id FROM mdl_blog_external b where b.id = mdl_post...
        ^
        HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts.
        DELETE FROM mdl_post
        WHERE mdl_post.module = 'blog_external'
        AND NOT EXISTS (SELECT b.id FROM mdl_blog_external b where b.id = mdl_post.content)
        AND ( NOT (uniquehash = '') )
        [array (
        )]
        Error code: dmlwriteexception
        Stack trace:
        line 410 of /lib/dml/moodle_database.php: dml_write_exception thrown
        line 239 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end()
        line 631 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end()
        line 1193 of /lib/db/upgrade.php: call to pgsql_native_moodle_database->execute()
        line 1483 of /lib/upgradelib.php: call to xmldb_main_upgrade()
        line 271 of /admin/index.php: call to upgrade_core()

        Show
        Aparup Banerjee added a comment - the upgrade bit is giving me this under postgres: Debug info: ERROR: operator does not exist: bigint = text LINE 3: ... (SELECT b.id FROM mdl_blog_external b where b.id = mdl_post... ^ HINT: No operator matches the given name and argument type(s). You might need to add explicit type casts. DELETE FROM mdl_post WHERE mdl_post.module = 'blog_external' AND NOT EXISTS (SELECT b.id FROM mdl_blog_external b where b.id = mdl_post.content) AND ( NOT (uniquehash = '') ) [array ( )] Error code: dmlwriteexception Stack trace: line 410 of /lib/dml/moodle_database.php: dml_write_exception thrown line 239 of /lib/dml/pgsql_native_moodle_database.php: call to moodle_database->query_end() line 631 of /lib/dml/pgsql_native_moodle_database.php: call to pgsql_native_moodle_database->query_end() line 1193 of /lib/db/upgrade.php: call to pgsql_native_moodle_database->execute() line 1483 of /lib/upgradelib.php: call to xmldb_main_upgrade() line 271 of /admin/index.php: call to upgrade_core()
        Aparup Banerjee made changes -
        Testing Instructions For this test you'll need to be comfortable with SQL.

        Log in as admin.
        Go to the external blogs screen (my profile settings > blogs > External blogs)
        Add two blogs. Each region covered by the BBC has its own RSS feed if you need some to test with. http://www.bbc.co.uk/news/world/asia/
        Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content.
        Through the external blog admin page delete one of the external blogs.
        Check that its posts are gone but that the posts from the other blog are still there.
        For this test you'll need to be comfortable with SQL. Test across all the supported DB drivers.

        Log in as admin.
        Go to the external blogs screen (my profile settings > blogs > External blogs)
        Add two blogs. Each region covered by the BBC has its own RSS feed if you need some to test with. http://www.bbc.co.uk/news/world/asia/
        Examine the 'blog_external' and the 'post' table. Posts from the 2 blogs will have been imported. The blog_external ID appears in post::content.
        Through the external blog admin page delete one of the external blogs.
        Check that its posts are gone but that the posts from the other blog are still there.
        Hide
        Aparup Banerjee added a comment -

        note: its fine for mysql and mssql for me.

        Show
        Aparup Banerjee added a comment - note: its fine for mysql and mssql for me.
        Hide
        Aparup Banerjee added a comment -

        trying with sql_cast_char2int()

        Show
        Aparup Banerjee added a comment - trying with sql_cast_char2int()
        Hide
        Eloy Lafuente (stronk7) added a comment -

        yeah, sql_compare_text() is to be able to compare varchar & text columns, not ints at all, for that, explicit cast is needed for sure.

        Show
        Eloy Lafuente (stronk7) added a comment - yeah, sql_compare_text() is to be able to compare varchar & text columns, not ints at all, for that, explicit cast is needed for sure.
        Hide
        Eloy Lafuente (stronk7) added a comment -

        offtopic, just looked for sql_compare_text() ocurrences in /blog and it's plenty of "incorrect" ones. Practically all should be casting ones. We really need to move those integers out from that column asap, grrr.

        Anyway, to get this fixed... we need to do something like this:

        
        index 7447298..5c4d234 100644
        --- a/lib/db/upgrade.php
        +++ b/lib/db/upgrade.php
        @@ -1185,7 +1185,7 @@ function xmldb_main_upgrade($oldversion) {
             }
         
             if ($oldversion < 2012090400.00) {
        -        $subquery = 'SELECT b.id FROM {blog_external} b where ' . $DB->sql_compare_text('b.id') . ' = ' . $DB->sql_compare_text('{post}.content');
        +        $subquery = 'SELECT b.id FROM {blog_external} b where b.id = ' . $DB->sql_cast_char2int('{post}.content', true);
                 $sql = 'DELETE FROM {post}
                               WHERE {post}.module = \'blog_external\'
                                     AND NOT EXISTS (' . $subquery . ')
        

        and cross the fingers about not finding non-integer values in that "contents" column or it will be breaking badly too.

        Show
        Eloy Lafuente (stronk7) added a comment - offtopic, just looked for sql_compare_text() ocurrences in /blog and it's plenty of "incorrect" ones. Practically all should be casting ones. We really need to move those integers out from that column asap, grrr. Anyway, to get this fixed... we need to do something like this: index 7447298..5c4d234 100644 --- a/lib/db/upgrade.php +++ b/lib/db/upgrade.php @@ -1185,7 +1185,7 @@ function xmldb_main_upgrade($oldversion) { } if ($oldversion < 2012090400.00) { - $subquery = 'SELECT b.id FROM {blog_external} b where ' . $DB->sql_compare_text('b.id') . ' = ' . $DB->sql_compare_text('{post}.content'); + $subquery = 'SELECT b.id FROM {blog_external} b where b.id = ' . $DB->sql_cast_char2int('{post}.content', true ); $sql = 'DELETE FROM {post} WHERE {post}.module = \'blog_external\' AND NOT EXISTS (' . $subquery . ') and cross the fingers about not finding non-integer values in that "contents" column or it will be breaking badly too.
        Eloy Lafuente (stronk7) made changes -
        Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
        Hide
        Eloy Lafuente (stronk7) added a comment -

        failing coz it's clear it does not work under, at least, postgres.

        Show
        Eloy Lafuente (stronk7) added a comment - failing coz it's clear it does not work under, at least, postgres.
        Eloy Lafuente (stronk7) made changes -
        Status Testing in progress [ 10011 ] Problem during testing [ 10007 ]
        Aparup Banerjee made changes -
        Status Problem during testing [ 10007 ] Integration review in progress [ 10004 ]
        Hide
        Aparup Banerjee added a comment - - edited

        ok i've pushed up a commit into 22, 23 and master in integration.git.

        mssql, pg and mysql fine here atm. (upgrades)

        Show
        Aparup Banerjee added a comment - - edited ok i've pushed up a commit into 22, 23 and master in integration.git. mssql, pg and mysql fine here atm. (upgrades)
        Aparup Banerjee made changes -
        Status Integration review in progress [ 10004 ] Waiting for testing [ 10005 ]
        Hide
        Michael de Raadt added a comment - - edited

        Just noting that the old code broke on Oracle, but it works after Apu's fix.

        This was during an upgrade on another issue. I wasn't specifically testing this issue.

        Show
        Michael de Raadt added a comment - - edited Just noting that the old code broke on Oracle, but it works after Apu's fix. This was during an upgrade on another issue. I wasn't specifically testing this issue.
        Michael de Raadt made changes -
        Tester rwijaya
        Rossiani Wijaya made changes -
        Status Waiting for testing [ 10005 ] Testing in progress [ 10011 ]
        Hide
        Michael de Raadt added a comment -

        I've now tested this under Oracle and MSSQL.

        Show
        Michael de Raadt added a comment - I've now tested this under Oracle and MSSQL.
        Hide
        Rossiani Wijaya added a comment -

        Thank you Michael for your help testing this issue.

        I tested on mysql and postgres, both DB works great.

        Test passed.

        Show
        Rossiani Wijaya added a comment - Thank you Michael for your help testing this issue. I tested on mysql and postgres, both DB works great. Test passed.
        Rossiani Wijaya made changes -
        Status Testing in progress [ 10011 ] Tested [ 10006 ]
        Hide
        Eloy Lafuente (stronk7) added a comment -

        Many thanks for the hard work.

        These changes have been spread upstream and are already available in the git and cvs repositories.

        Ciao

        Show
        Eloy Lafuente (stronk7) added a comment - Many thanks for the hard work. These changes have been spread upstream and are already available in the git and cvs repositories. Ciao
        Eloy Lafuente (stronk7) made changes -
        Status Tested [ 10006 ] Closed [ 6 ]
        Resolution Fixed [ 1 ]
        Currently in integration Yes [ 10041 ]
        Integration date 07/Sep/12
        Eloy Lafuente (stronk7) made changes -
        Fix Version/s STABLE Sprint 23 Alpha [ 12358 ]

          People

          • Votes:
            2 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: