Doctrine 1
  1. Doctrine 1
  2. DC-727

ReOpen DC-46 - Unexpected behavior with whereIn() and empty array

    Details

    • Type: Bug Bug
    • Status: Open
    • Priority: Major Major
    • Resolution: Unresolved
    • Affects Version/s: 1.2.1, 1.2.2, 1.2.3
    • Fix Version/s: None
    • Component/s: Query
    • Labels:
      None
    • Environment:

      Description

      I reopen the DC-46 as it'seems not fix at all.
      When I do a whereIn with empty array, the condition is just drop and I get no Exception.

      Here is a simple test case:

      require_once('doctrine/lib/Doctrine.php');
      spl_autoload_register(array('Doctrine', 'autoload'));
      $manager = Doctrine_Manager::getInstance();
      $manager->setAttribute(Doctrine::ATTR_EXPORT, Doctrine::EXPORT_ALL);
      $conn = Doctrine_Manager::connection('mysql://root:root@localhost/test_doctrine');
      echo "Connection is set up\n";
      
      class Record extends Doctrine_Record {
          public function setTableDefinition(){
              $this->setTableName('record');
          }
      }
      
      // Create the db
      try {Doctrine::dropDatabases();}catch(Exception $e){} // Drop if exist :-)
      Doctrine::createDatabases();Doctrine::createTablesFromArray(array('Record'));
      
      // Test
      echo Doctrine::getTable('Record')->createQuery()->select('id')->whereIn('id', array())->getSqlQuery() , "\n";
      echo Doctrine::getTable('Record')->createQuery()->select('id')->whereIn('id', array())->fetchArray() , "\n";
      
      // Result is:
      // SELECT r.id AS r__id FROM record r
      // Array
      
      1. DC-727_refactored.patch
        2 kB
        Guilliam X
      2. DC-727_with_duplicates.patch
        2 kB
        Guilliam X
      3. DC727TestCase.php
        1 kB
        Guilliam X
      4. DC727TestCase_more_specific.php
        1 kB
        Guilliam X

        Activity

        Hide
        Guilliam X added a comment -

        The problem is that the change for "new" behavior (throw exception instead of return unchanged query) was only done in _processWhereIn() but not cascaded to andWhereIn() and orWhereIn() (another example we should avoid code duplication).
        The patch is simple, but it causes Doctrine_Ticket_1558_TestCase to fail. Indeed that (old) test expects the "old" behavior (return unchanged query, don't throw exception)... So the 2 fixes are incompatible, you'll have to choose :/

        I still attach a new test case and 2 versions of a patch (the first one just applies changes of _processWhereIn also to the 2 other functions but adds more duplicate code, the second is a little refactored and seems better to me).

        Regards

        Show
        Guilliam X added a comment - The problem is that the change for "new" behavior (throw exception instead of return unchanged query) was only done in _ processWhereIn() but not cascaded to andWhereIn() and orWhereIn() (another example we should avoid code duplication). The patch is simple, but it causes Doctrine_Ticket_1558_TestCase to fail. Indeed that (old) test expects the "old" behavior (return unchanged query, don't throw exception)... So the 2 fixes are incompatible, you'll have to choose :/ I still attach a new test case and 2 versions of a patch (the first one just applies changes of _processWhereIn also to the 2 other functions but adds more duplicate code, the second is a little refactored and seems better to me). Regards
        Hide
        Guilliam X added a comment -

        added a more specific test case (expects Doctrine_Query_Exception instead of simple Exception)

        Show
        Guilliam X added a comment - added a more specific test case (expects Doctrine_Query_Exception instead of simple Exception)
        Hide
        Jan Schütze added a comment -

        Hi,

        the issue is still present in 1.2.3. Are there any plans to apply it?

        Kind regards,

        Show
        Jan Schütze added a comment - Hi, the issue is still present in 1.2.3. Are there any plans to apply it? Kind regards,
        Hide
        Jim Persson added a comment -

        I would like to add that this also applies to delete-queries which can cause serious data loss.
        We had a case where a table of serialized data was completely emptied which caused a database cascade that deleted quite a lot of data.

        Show
        Jim Persson added a comment - I would like to add that this also applies to delete-queries which can cause serious data loss. We had a case where a table of serialized data was completely emptied which caused a database cascade that deleted quite a lot of data.

          People

          • Assignee:
            Jonathan H. Wage
            Reporter:
            David Jeanmonod
          • Votes:
            3 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated: