Details
-
Type:
Bug
-
Status:
Open
-
Priority:
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:HidePHP 5.3.1 (cli) (built: Feb 11 2010 02:32:22)
mysql Ver 14.14 Distrib 5.1.41, for apple-darwin9.5.0 (i386) using readline 5.1
Doctrine version 1.2.2 from SVN: http://doctrine.mirror.svn.symfony-project.com/tags/1.2.2/lib/Doctrine.phpShowPHP 5.3.1 (cli) (built: Feb 11 2010 02:32:22) mysql Ver 14.14 Distrib 5.1.41, for apple-darwin9.5.0 (i386) using readline 5.1 Doctrine version 1.2.2 from SVN: http://doctrine.mirror.svn.symfony-project.com/tags/1.2.2/lib/Doctrine.php
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
Activity
David Jeanmonod
made changes -
| Field | Original Value | New Value |
|---|---|---|
| Affects Version/s | 1.2.2 [ 10047 ] | |
| Environment |
PHP 5.3.1 (cli) (built: Feb 11 2010 02:32:22) mysql Ver 14.14 Distrib 5.1.41, for apple-darwin9.5.0 (i386) using readline 5.1 Doctrine version 1.2.2 from SVN: http://doctrine.mirror.svn.symfony-project.com/tags/1.2.2/lib/Doctrine.php |
|
| Issue Type | Improvement [ 4 ] | Bug [ 1 ] |
| Description |
See trac #2064 ( http://trac.doctrine-project.org/ticket/2064 ) Jwage wrote "This is expected behavior. We cannot and will not add a fake invalid condition to the where string. I think we need to make it throw an exception if you try and pass an empty array." I agree that an exception should be thrown. But AFAIK this hasn't been implemented yet. The issue is quite urgent in my opinion. Example: I had a query where I absolutely had to restrict the result set to allowed objects (IDs) only. I used a whereIn() condition for this restriction. Problem is: when the user wasn't assigned to any groups, the id-array was empty, and Doctrine removed the WHERE ... IN completely! That resulted in a result set containing all records! I would expect - either to return no records (correct behavior could be to add "0=1" as a replacement for "... IN ()" expression) - or let the database decide (= submitting the query with an empty list which would result - at least for MySQL - in a database exception) - or Doctrine could throw an exception Would IMO be way better (as its IMO a more expected behavior) than simply not adding the "... IN ()" to the query. |
I reopen the When I do a whereIn with empty array, the condition is just drop and I get no Exception. Here is a simple test case: {code} 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 {code} |
Guilliam X
made changes -
| Attachment | DC727TestCase.php [ 10657 ] | |
| Attachment | DC-727_with_duplicates.patch [ 10658 ] | |
| Attachment | DC-727_refactored.patch [ 10659 ] |
Guilliam X
made changes -
| Attachment | DC727TestCase_more_specific.php [ 10660 ] |
Roman S. Borschel
made changes -
| Assignee | Roman S. Borschel [ romanb ] | Jonathan H. Wage [ jwage ] |
Jan Schütze
made changes -
| Affects Version/s | 1.2.3 [ 10051 ] | |
| Affects Version/s | 1.2.1 [ 10044 ] |
This list may be incomplete, as errors occurred whilst retrieving source from linked applications:
- Request to http://www.doctrine-project.org/fisheye/ failed: Error in remote call to 'FishEye 0 (http://www.doctrine-project.org/fisheye/)' (http://www.doctrine-project.org/fisheye) [AbstractRestCommand{path='/rest-service-fe/search-v1/crossRepositoryQuery', params={query=DC-727, expand=changesets[-21:-1].revisions[0:29],reviews}, methodType=GET}] : Received status code 503 (Service Temporarily Unavailable)
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