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

      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.

        Gliffy Diagrams

          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: