Refactoring. Started to introduced a clear separation between column names and...

Refactoring. Started to introduced a clear separation between column names and field names (aka column aliases).\nDoctrine_Connection, DQL Parser classes/methods map field names => column names.\nDoctrine_Hydrate maps column names => field names during hydration.\nColumn names are only stored in Doctrine_Table:: and Doctrine_Table::.\nRelations use column names in 'local'/'foreign'.\nWhen using field names (column aliases) you need to use the column names in 'local'/'foreign' when setting up a relation (hasOne/hasMany), not the field names.\n\n In other words column names are only used to communicate with the database. field names are used everywhere else. the casing of field names does not matter. column names are forced to lower case for portability. If you dont use field names (column aliases) your column names are your field names (and therefore all lowercase).

CHANGELOG

 Beta 2
 ------
 
+* r3183: NestedSet: 'level' column renamed to 'lvl' because LEVEL is an oracle keyword.
+         In order to upgrade existing trees you need to rename the level column to lvl
+         in your databases. This does not affect your code because the NestedSet now uses
+         a column alias (lvl as level). So your code still refers to the 'level' field.
 * r3048: Doctrine::exportSchema() replaced by Doctrine::createTablesFromModels()
 * r3048: Doctrine::exportSql() replaced by Doctrine::generateSqlFromModels()
 * r3048: Doctrine::importSchema() replaced by Doctrine::generateModelsFromDb()

lib/Doctrine/Connection.php

@@ -151,7 +151,7 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
     protected $options    = array();
 
     /**
-     * @var array $availableDrivers         an array containing all availible drivers
+     * @var array $availableDrivers         an array containing all available drivers
      */
     private static $availableDrivers    = array(
                                         'Mysql',
@@ -180,7 +180,7 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
 
             $this->isConnected = true;
 
-        } elseif (is_array($adapter)) {
+        } else if (is_array($adapter)) {
             $this->pendingAttributes[Doctrine::ATTR_DRIVER_NAME] = $adapter['scheme'];
 
             $this->options['dsn']      = $adapter['dsn'];
@@ -468,7 +468,7 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @throws PDOException                         if something fails at PDO level
      * @return integer                              number of rows affected
      */
-    public function replace($table, array $fields, array $keys)
+    public function replace(Doctrine_Table $table, array $fields, array $keys)
     {
         //if ( ! $this->supports('replace'))
         //    throw new Doctrine_Connection_Exception('replace query is not supported');
@@ -478,26 +478,26 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
         }
         $condition = $values = array();
 
-        foreach ($fields as $name => $value) {
-            $values[$name] = $value;
+        foreach ($fields as $fieldName => $value) {
+            $values[$fieldName] = $value;
 
-            if (in_array($name, $keys)) {
+            if (in_array($fieldName, $keys)) {
                 if ($value === null)
-                    throw new Doctrine_Connection_Exception('key value '.$name.' may not be null');
+                    throw new Doctrine_Connection_Exception('key value '.$fieldName.' may not be null');
 
-                $condition[]       = $name . ' = ?';
+                $condition[] = $table->getColumnName($fieldName) . ' = ?';
                 $conditionValues[] = $value;
             }
         }
 
-        $query          = 'DELETE FROM ' . $this->quoteIdentifier($table) . ' WHERE ' . implode(' AND ', $condition);
-        $affectedRows   = $this->exec($query);
+        $query = 'DELETE FROM ' . $this->quoteIdentifier($table->getTableName())
+                . ' WHERE ' . implode(' AND ', $condition);
+        $affectedRows = $this->exec($query, $conditionValues);
 
         $this->insert($table, $values);
 
         $affectedRows++;
 
-
         return $affectedRows;
     }
 
@@ -509,16 +509,16 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @param array $identifier     An associateve array containing identifier column-value pairs.
      * @return integer              The number of affected rows
      */
-    public function delete($table, array $identifier)
+    public function delete(Doctrine_Table $table, array $identifier)
     {
         $tmp = array();
 
         foreach (array_keys($identifier) as $id) {
-            $tmp[] = $id . ' = ?';
+            $tmp[] = $table->getColumnName($id) . ' = ?';
         }
 
         $query = 'DELETE FROM '
-               . $this->quoteIdentifier($table)
+               . $this->quoteIdentifier($table->getTableName())
                . ' WHERE ' . implode(' AND ', $tmp);
 
 
@@ -534,27 +534,27 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @return mixed            boolean false if empty value array was given,
      *                          otherwise returns the number of affected rows
      */
-    public function update($table, array $values, array $identifier)
+    public function update(Doctrine_Table $table, array $fields, array $identifier)
     {
-        if (empty($values)) {
+        if (empty($fields)) {
             return false;
         }
 
         $set = array();
-        foreach ($values as $name => $value) {
+        foreach ($fields as $fieldName => $value) {
             if ($value instanceof Doctrine_Expression) {
-                $set[] = $name . ' = ' . $value->getSql();
+                $set[] = $table->getColumnName($fieldName) . ' = ' . $value->getSql();
                 unset($values[$name]);
             } else {
-                $set[] = $name . ' = ?';
+                $set[] = $table->getColumnName($fieldName) . ' = ?';
             }
         }
 
-        $params = array_merge(array_values($values), array_values($identifier));
+        $params = array_merge(array_values($fields), array_values($identifier));
 
-        $sql  = 'UPDATE ' . $this->quoteIdentifier($table)
+        $sql  = 'UPDATE ' . $this->quoteIdentifier($table->getTableName())
               . ' SET ' . implode(', ', $set)
-              . ' WHERE ' . implode(' = ? AND ', array_keys($identifier))
+              . ' WHERE ' . implode(' = ? AND ', $table->getIdentifierColumnNames())
               . ' = ?';
 
         return $this->exec($sql, $params);
@@ -568,34 +568,36 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
      * @return mixed            boolean false if empty value array was given,
      *                          otherwise returns the number of affected rows
      */
-    public function insert($table, array $values) {
-        if (empty($values)) {
+    public function insert(Doctrine_Table $table, array $fields) {
+        if (empty($fields)) {
             return false;
         }
 
+        $tableName = $table->getTableName();
+
         // column names are specified as array keys
         $cols = array();
         // the query VALUES will contain either expresions (eg 'NOW()') or ?
         $a = array();
-        foreach ($values as $k => $value) {
-            $cols[] = $this->quoteIdentifier($k);
+        foreach ($fields as $fieldName => $value) {
+            $cols[] = $this->quoteIdentifier($table->getColumnName($fieldName));
             if ($value instanceof Doctrine_Expression) {
                 $a[] = $value->getSql();
-                unset($values[$k]);
+                unset($fields[$fieldName]);
             } else {
                 $a[] = '?';
             }
         }
 
         // build the statement
-        $query = 'INSERT INTO ' . $this->quoteIdentifier($table) 
+        $query = 'INSERT INTO ' . $this->quoteIdentifier($tableName) 
                . ' (' . implode(', ', $cols) . ') '
                . 'VALUES (';
 
         $query .= implode(', ', $a) . ')';
         // prepare and execute the statement
 
-        return $this->exec($query, array_values($values));
+        return $this->exec($query, array_values($fields));
     }
 
     /**
@@ -912,6 +914,7 @@ abstract class Doctrine_Connection extends Doctrine_Configurable implements Coun
                 $this->getAttribute(Doctrine::ATTR_LISTENER)->preQuery($event);
 
                 if ( ! $event->skipOperation) {
+                    //echo $query . "<br />";
                     $stmt = $this->dbh->query($query);
 
                     $this->_count++;

lib/Doctrine/Connection/Mysql.php

@@ -166,7 +166,7 @@ class Doctrine_Connection_Mysql extends Doctrine_Connection_Common
      *
      * @return integer      the number of affected rows
      */
-    public function replace($table, array $fields, array $keys)
+    public function replace(Doctrine_Table $table, array $fields, array $keys)
     {
         $count = count($fields);
         $query = $values = '';
@@ -180,7 +180,7 @@ class Doctrine_Connection_Mysql extends Doctrine_Connection_Common
                 $values.= ',';
             }
 
-            $query .= $name;
+            $query .= $table->getColumnName($name);
 
             if (isset($fields[$name]['null']) && $fields[$name]['null']) {
                 $value = 'NULL';
@@ -202,7 +202,7 @@ class Doctrine_Connection_Mysql extends Doctrine_Connection_Common
         if ($keys == 0) {
             throw new Doctrine_Connection_Mysql_Exception('not specified which fields are keys');
         }
-        $query = 'REPLACE INTO ' . $table . ' (' . $query . ') VALUES (' . $values . ')';
+        $query = 'REPLACE INTO ' . $table->getTableName() . ' (' . $query . ') VALUES (' . $values . ')';
 
         return $this->exec($query);
     }

lib/Doctrine/Connection/UnitOfWork.php

@@ -148,9 +148,8 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
         }
 
         $record->state(Doctrine_Record::STATE_LOCKED);
-
+        
         $conn->beginTransaction();
-
         $saveLater = $this->saveRelated($record);
 
         $record->state($state);
@@ -159,7 +158,7 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
             $event = new Doctrine_Event($record, Doctrine_Event::RECORD_SAVE);
 
             $record->preSave($event);
-    
+
             $record->getTable()->getRecordListener()->preSave($event);
             $state = $record->state();
 
@@ -180,7 +179,7 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
             }
 
             $record->getTable()->getRecordListener()->postSave($event);
-            
+             
             $record->postSave($event);
         } else {
             $conn->transaction->addInvalid($record);
@@ -195,7 +194,7 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
 
             if ($record->hasReference($alias)) {
                 $obj = $record->$alias;
-                
+            
                 // check that the related object is not an instance of Doctrine_Null
                 if ( ! ($obj instanceof Doctrine_Null)) {
                     $obj->save($conn);
@@ -207,7 +206,7 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
         $this->saveAssociations($record);
 
         $record->state($state);
-
+        
         $conn->commit();
 
         return true;
@@ -284,10 +283,10 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
                 foreach ($table->getOption('joinedParents') as $parent) {
                     $parentTable = $table->getConnection()->getTable($parent);
                     
-                    $this->conn->delete($parentTable->getTableName(), $record->identifier());
+                    $this->conn->delete($parentTable, $record->identifier());
                 }
             }
-            $this->conn->delete($table->getTableName(), $record->identifier());
+            $this->conn->delete($table, $record->identifier());
 
             $record->state(Doctrine_Record::STATE_TCLEAN);
         } else {
@@ -295,7 +294,6 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
             $record->state($state);
         }
 
-
         $table->getRecordListener()->postDelete($event);
 
         $record->postDelete($event);
@@ -554,12 +552,12 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
                 foreach ($classes as $class) {
                     $parentTable = $this->conn->getTable($class);
 
-                    $this->conn->update($this->conn->getTable($class)->getTableName(), $dataSet[$class], $identifier);
+                    $this->conn->update($this->conn->getTable($class), $dataSet[$class], $identifier);
                 }
             } else {
                 $array = $record->getPrepared();
                 
-                $this->conn->update($table->getTableName(), $array, $identifier);
+                $this->conn->update($table, $array, $identifier);
             }
             $record->assignIdentifier(true);
         }
@@ -608,7 +606,7 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
                             $dataSet[$parent][$id] = $value;
                         }
 
-                        $this->conn->insert($this->conn->getTable($parent)->getTableName(), $dataSet[$parent]);
+                        $this->conn->insert($this->conn->getTable($parent), $dataSet[$parent]);
                     }
                 }
             } else {
@@ -624,6 +622,10 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
 
         return true;
     }
+    
+    /**
+     * @todo DESCRIBE WHAT THIS METHOD DOES, PLEASE!
+     */
     public function formatDataSet(Doctrine_Record $record)
     {
     	$table = $record->getTable();
@@ -634,47 +636,53 @@ class Doctrine_Connection_UnitOfWork extends Doctrine_Connection_Module
     
         $array = $record->getPrepared();
     
-        foreach ($table->getColumns() as $column => $definition) {
+        foreach ($table->getColumns() as $columnName => $definition) {
+            $fieldName = $table->getFieldName($columnName);
             if (isset($definition['primary']) && $definition['primary']) {
                 continue;
             }
     
             if (isset($definition['owner'])) {
-                $dataSet[$definition['owner']][$column] = $array[$column];
+                $dataSet[$definition['owner']][$fieldName] = $array[$fieldName];
             } else {
-                $dataSet[$component][$column] = $array[$column];
+                $dataSet[$component][$fieldName] = $array[$fieldName];
             }
         }    
         
         return $dataSet;
     }
+    
+    /**
+     * @todo DESCRIBE WHAT THIS METHOD DOES, PLEASE!
+     */
     public function processSingleInsert(Doctrine_Record $record)
     {
-        $array = $record->getPrepared();
+        $fields = $record->getPrepared();
 
-        if (empty($array)) {
+        if (empty($fields)) {
             return false;
         }
-        $table     = $record->getTable();
-        $keys      = (array) $table->getIdentifier();
+        
+        $table = $record->getTable();
+        $identifier = (array) $table->getIdentifier();
 
-        $seq       = $record->getTable()->sequenceName;
+        $seq = $record->getTable()->sequenceName;
 
         if ( ! empty($seq)) {
-            $id             = $this->conn->sequence->nextId($seq);
-            $name           = $record->getTable()->getIdentifier();
-            $array[$name]   = $id;
+            $id = $this->conn->sequence->nextId($seq);
+            $seqName = $table->getIdentifier();
+            $fields[$seqName] = $id;
 
             $record->assignIdentifier($id);
         }
 
-        $this->conn->insert($table->getTableName(), $array);
+        $this->conn->insert($table, $fields);
 
-        if (empty($seq) && count($keys) == 1 && $keys[0] == $table->getIdentifier() &&
+        if (empty($seq) && count($identifier) == 1 && $identifier[0] == $table->getIdentifier() &&
             $table->getIdentifierType() != Doctrine::IDENTIFIER_NATURAL) {
 
             if (strtolower($this->conn->getName()) == 'pgsql') {
-                $seq = $table->getTableName() . '_' . $keys[0];
+                $seq = $table->getTableName() . '_' . $identifier[0];
             }
 
             $id = $this->conn->sequence->lastInsertId($seq);

lib/Doctrine/Hydrate.php

@@ -819,7 +819,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
         if ($this->type !== self::SELECT) {
             return $this->_conn->exec($query, $params);
         }
-
+        //echo $query . "<br /><br />";
         $stmt = $this->_conn->execute($query, $params);
         return $stmt;
     }
@@ -850,7 +850,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
             if ($cached === false) {
                 // cache miss
                 $stmt = $this->_execute($params);
-                $array = $this->parseData2($stmt, Doctrine::HYDRATE_ARRAY);
+                $array = $this->hydrateResultSet($stmt, Doctrine::HYDRATE_ARRAY);
 
                 $cached = $this->getCachedForm($array);
 
@@ -883,7 +883,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
                 return $stmt;
             }
             
-            $array = $this->parseData2($stmt, $hydrationMode);
+            $array = $this->hydrateResultSet($stmt, $hydrationMode);
         }
         return $array;
     }
@@ -1022,7 +1022,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
      * @param mixed $stmt
      * @return array
      */
-    public function parseData2($stmt, $hydrationMode)
+    public function hydrateResultSet($stmt, $hydrationMode)
     {
         if ($hydrationMode == Doctrine::HYDRATE_NONE) {
             return $stmt->fetchAll(PDO::FETCH_NUM);
@@ -1076,29 +1076,33 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
                 // of magnitude.
                 if ( ! isset($cache[$key])) {
                     $e = explode('__', $key);
-                    $cache[$key]['field'] = $field = strtolower(array_pop($e));
+                    $last = strtolower(array_pop($e));          
                     $cache[$key]['alias'] = $this->_tableAliases[strtolower(implode('__', $e))];
+                    $fieldName = $this->_aliasMap[$cache[$key]['alias']]['table']->getFieldName($last);
+                    //echo "hydrate:" . $fieldName . "<br /><br />";
+                    $cache[$key]['fieldName'] = $fieldName;
+                    $cache[$key]['columnName'] = $last;
                 }
 
                 $map   = $this->_aliasMap[$cache[$key]['alias']];
                 $table = $map['table'];
                 $alias = $cache[$key]['alias'];
-                $field = $cache[$key]['field'];
+                $fieldName = $cache[$key]['fieldName'];
 
-                if (isset($this->_aliasMap[$alias]['agg'][$field])) {
-                    $field = $this->_aliasMap[$alias]['agg'][$field];
+                if (isset($this->_aliasMap[$alias]['agg'][$fieldName])) {
+                    $fieldName = $this->_aliasMap[$alias]['agg'][$fieldName];
                 }
 
-
-                if ($table->isIdentifier($field)) {
+                if ($table->isIdentifier($fieldName)) {
                     $id[$alias] .= '|' . $value;
                 }
 
-                $currData[$alias][$field] = $table->prepareValue($field, $value);
+                $currData[$alias][$fieldName] = $table->prepareValue($fieldName, $value);
 
                 if ($value !== null) {
                     $identifiable[$alias] = true;
                 }
+                
             }
 
             // dealing with root component
@@ -1162,7 +1166,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
                 if ( ! isset($prev[$parent])) {
                     break;
                 }
-
+                
                 // check the type of the relation
                 if ( ! $relation->isOneToOne()) {
                     // initialize the collection
@@ -1207,7 +1211,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
                     } else {
                         $prev[$parent][$componentAlias] = $element;
                     }
-                    $oneToOne = true;
+                    $oneToOne = true; 
                 }
                 $coll =& $prev[$parent][$componentAlias];
                 $this->_setLastElement($prev, $coll, $index, $alias, $oneToOne);
@@ -1215,7 +1219,7 @@ class Doctrine_Hydrate extends Doctrine_Locator_Injectable implements Serializab
             }
             $id[$rootAlias] = '';
         }
-
+        
         $driver->flush();
 
         $stmt->closeCursor();

lib/Doctrine/Hydrate/Record.php

@@ -78,7 +78,7 @@ class Doctrine_Hydrate_Record extends Doctrine_Locator_Injectable
      */
     public function isIdentifiable(array $row, Doctrine_Table $table)
     {
-        $primaryKeys = $table->getIdentifier();
+        $primaryKeys = $table->getIdentifierColumnNames();
 
         if (is_array($primaryKeys)) {
             foreach ($primaryKeys as $id) {
@@ -103,8 +103,15 @@ class Doctrine_Hydrate_Record extends Doctrine_Locator_Injectable
             $this->_tables[$component] = Doctrine_Manager::getInstance()->getTable($component);
             $this->_tables[$component]->setAttribute(Doctrine::ATTR_LOAD_REFERENCES, false);
         }
+        
+        //echo "..before..";
+        //Doctrine::dump($data);
+        
         $this->_tables[$component]->setData($data);
         $record = $this->_tables[$component]->getRecord();
+        
+        //echo "..after..";
+        //Doctrine::dump($record->getData());
 
         if ( ! isset($this->_records[$record->getOid()]) ) {
             $record->clearRelated();

lib/Doctrine/Query.php

@@ -32,6 +32,7 @@ Doctrine::autoload('Doctrine_Query_Abstract');
  */
 class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 {
+    /** @todo document the query states (and the transitions between them). */
     const STATE_CLEAN  = 1;
 
     const STATE_DIRTY  = 2;
@@ -156,7 +157,10 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
     protected $_pendingJoinConditions = array();
 
     protected $_expressionMap = array();
-
+    
+    /**
+     * @var integer $_state   The current state of this query.
+     */
     protected $_state = Doctrine_Query::STATE_CLEAN;
 
     /**
@@ -170,7 +174,10 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
     {
         return new Doctrine_Query($conn);
     }
-
+    
+    /**
+     * Resets the query to the state just after it has been instantiated.
+     */
     public function reset()
     {
         $this->_pendingJoinConditions = array();
@@ -318,6 +325,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      *
      * @throws Doctrine_Query_Exception     if unknown parser name given
      * @return Doctrine_Query_Part
+     * @todo Doc/Description: What is the parameter for? Which parsers are available?
      */
     public function getParser($name)
     {
@@ -386,10 +394,12 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
     /**
      * getDqlPart
-     * returns the given DQL query part 
+     * returns a specific DQL query part.
      *
      * @param string $queryPart     the name of the query part
      * @return string   the DQL query part
+     * @todo Description: List which query parts exist or point to the method/property
+     *       where they are listed.
      */
     public function getDqlPart($queryPart)
     {
@@ -402,7 +412,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
     /**
      * getDql
-     * returns the DQL query associated with this object
+     * returns the DQL query that is represented by this query object.
      *
      * the query is built from $_dqlParts
      *
@@ -432,6 +442,8 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      * @throws Doctrine_Query_Exception     if unknown component alias has been given
      * @param string $componentAlias        the alias of the component
      * @return void
+     * @todo Description: What is a 'pending field' (and are there non-pending fields, too)?
+     *       What is 'processed'? (Meaning: What information is gathered & stored away)
      */
     public function processPendingFields($componentAlias)
     {
@@ -443,6 +455,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
             // check for wildcards
             if (in_array('*', $fields)) {
+                //echo "<br />";Doctrine::dump($table->getColumnNames()); echo "<br />";
                 $fields = $table->getColumnNames();
             } else {
                 // only auto-add the primary key fields if this query object is not
@@ -477,7 +490,8 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
             }
 
             $this->_neededTables[] = $tableAlias;
-
+            //Doctrine::dump(implode(', ', $sql));
+            //echo "<br /><br />";
             return implode(', ', $sql);
         }
     }
@@ -487,6 +501,9 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      *
      * @throws Doctrine_Query_Exception     if unknown component alias has been given
      * @return void
+     * @todo Description: Explain what this method does. Is there a relation to parseSelect()?
+     *       (It doesnt seem to get called from there...?). In what circumstances is this method
+     *       used?
      */
     public function parseSelectField($field)
     {
@@ -533,6 +550,8 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      *
      * @param string $expr      expression from which to get to owner from
      * @return string           the component alias
+     * @todo Description: What does it mean if a component is an 'owner' of an expression?
+     *       What kind of 'expression' are we talking about here?
      */
     public function getExpressionOwner($expr)
     {
@@ -557,6 +576,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      * adds selected fields to pendingFields array
      *
      * @param string $dql
+     * @todo Description: What information is extracted (and then stored)?
      */
     public function parseSelect($dql)
     {
@@ -640,8 +660,10 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      * 5. Parses nested clauses and subqueries recursively
      *
      * @return string   SQL string
+     * @todo Description: What is a 'dql clause' (and what not)?
+     *       Refactor: Too long & nesting level
      */
-    public function parseClause($clause) 
+    public function parseClause($clause)
     {
     	$clause = trim($clause);
 
@@ -670,7 +692,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
                     // convert DQL function to its RDBMS specific equivalent
                     try {
                         $expr = call_user_func_array(array($this->_conn->expression, $name), $args);
-                    } catch(Doctrine_Expression_Exception $e) {
+                    } catch (Doctrine_Expression_Exception $e) {
                         throw new Doctrine_Query_Exception('Unknown function ' . $expr . '.');
                     }
                     $term[0] = $expr;
@@ -849,6 +871,8 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      * since some subqueries may be correlated
      *
      * @return void
+     * @todo Better description. i.e. What is a 'pending subquery'? What does 'processed' mean?
+     *       (parsed? sql is constructed? some information is gathered?)
      */
     public function processPendingSubqueries()
     {
@@ -878,6 +902,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      * processes pending aggregate values for given component alias
      *
      * @return void
+     * @todo Better description. i.e. What is a 'pending aggregate'? What does 'processed' mean?
      */
     public function processPendingAggregates()
     {
@@ -1135,7 +1160,8 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
         $table = $map['table'];
         $rootAlias = key($this->_aliasMap);
 
-        if ( ! empty($this->parts['limit']) && $this->needsSubquery && $table->getAttribute(Doctrine::ATTR_QUERY_LIMIT) == Doctrine::LIMIT_RECORDS) {
+        if ( ! empty($this->parts['limit']) && $this->needsSubquery &&
+                $table->getAttribute(Doctrine::ATTR_QUERY_LIMIT) == Doctrine::LIMIT_RECORDS) {
             $this->isLimitSubqueryUsed = true;
             $needsSubQuery = true;
         }
@@ -1235,6 +1261,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      * of limiting the number of sql result set rows
      *
      * @return string       the limit subquery
+     * @todo A little refactor to make the method easier to understand & maybe shorter?
      */
     public function getLimitSubquery()
     {
@@ -1445,7 +1472,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
         $parts = $this->tokenizeQuery($query);
 
-        foreach($parts as $k => $part) {
+        foreach ($parts as $k => $part) {
             $part = implode(' ', $part);
             $k = strtolower($k);
             switch ($k) {
@@ -1485,7 +1512,10 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
         return $this;
     }
-
+    
+    /**
+     * @todo DESCRIBE ME! REFACTOR ME! I'M FAR TOO LONG AND COMPLEX! HARD TO UNDERSTAND!
+     */
     public function load($path, $loadFields = true)
     {
     	if (isset($this->_aliasMap[$path])) {
@@ -1610,7 +1640,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
                     $queryPart .= ' ON ' . $localAlias
                                 . '.'
-                                . $localTable->getIdentifier()
+                                . $localTable->getIdentifier() // what about composite keys?
                                 . ' = '
                                 . $assocAlias . '.' . $relation->getLocal();
 
@@ -1708,6 +1738,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
      *
      * @param string $name
      * @param string $componentAlias
+     * @todo DESCRIBE ME!
      */
     public function loadRoot($name, $componentAlias)
     {
@@ -1737,11 +1768,14 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
 
         return $table;
     }
+    
+    /**
+     * @todo DESCRIBE ME!
+     */
     public function buildInheritanceJoinSql($name, $componentAlias)
     {
         // get the connection for the component
-        $this->_conn = Doctrine_Manager::getInstance()
-                      ->getConnectionForComponent($name);
+        $this->_conn = Doctrine_Manager::getInstance()->getConnectionForComponent($name);
 
         $table = $this->_conn->getTable($name);
         $tableName = $table->getTableName();
@@ -1762,6 +1796,7 @@ class Doctrine_Query extends Doctrine_Query_Abstract implements Countable
             $queryPart .= ' LEFT JOIN ' . $this->_conn->quoteIdentifier($parentTable->getTableName())
                         . ' ' . $this->_conn->quoteIdentifier($parentTableAlias) . ' ON ';
             
+            //Doctrine::dump($table->getIdentifier());
             foreach ((array) $table->getIdentifier() as $identifier) {
                 $column = $table->getColumnName($identifier);
 

lib/Doctrine/Transaction.php

@@ -56,6 +56,7 @@ class Doctrine_Transaction extends Doctrine_Connection_Module
 
     /**
      * @var array $invalid                  an array containing all invalid records within this transaction
+     * @todo What about a more verbose name? $invalidRecords?
      */
     protected $invalid          = array();
 
@@ -294,12 +295,19 @@ class Doctrine_Transaction extends Doctrine_Connection_Module
      * @param string $savepoint                 name of a savepoint to rollback to   
      * @throws Doctrine_Transaction_Exception   if the rollback operation fails at database level
      * @return boolean                          false if rollback couldn't be performed, true otherwise
+     * @todo Shouldnt this method only commit a rollback if the transactionLevel is 1
+     *       (STATE_ACTIVE)? Explanation: Otherwise a rollback that is triggered from inside doctrine
+     *       in an (emulated) nested transaction would lead to a complete database level
+     *       rollback even though the client code did not yet want to do that.
+     *       In other words: if the user starts a transaction doctrine shouldnt roll it back.
+     *       Doctrine should only roll back transactions started by doctrine. Thoughts?
      */
     public function rollback($savepoint = null)
     {
         $this->conn->connect();
-
-        if ($this->transactionLevel == 0) {
+        $currentState = $this->getState();
+        
+        if ($currentState != self::STATE_ACTIVE && $currentState != self::STATE_BUSY) {
             return false;
         }
 

models/ColumnAliasTest.php

@@ -4,9 +4,12 @@ class ColumnAliasTest extends Doctrine_Record
     public function setTableDefinition()
     {
         $this->hasColumn('column1 as alias1', 'string', 200);
-        $this->hasColumn('column2 as alias2', 'integer', 11);
+        $this->hasColumn('column2 as alias2', 'integer', 4);
+        $this->hasColumn('another_column as anotherField', 'string', 50);
+        $this->hasColumn('book_id as bookId', 'integer', 4);
     }
-    public function setUp() 
+    public function setUp()
     {
+        $this->hasOne('Book as book', array('local' => 'book_id', 'foreign' => 'id'));
     }
 }

tests/AccessTestCase.php

@@ -48,7 +48,7 @@ class Doctrine_Access_TestCase extends Doctrine_UnitTestCase
     public function testIsset() 
     {
         $user = new User();
-        
+
         $this->assertTrue(isset($user->name));  
         $this->assertFalse(isset($user->unknown));
         
@@ -70,7 +70,7 @@ class Doctrine_Access_TestCase extends Doctrine_UnitTestCase
     public function testOffsetMethods() 
     {
         $user = new User();
-        $this->assertEqual($user['name'],null);
+        $this->assertEqual($user['name'], null);
 
         $user['name'] = 'Jack';
         $this->assertEqual($user['name'], 'Jack');

tests/ClassTableInheritanceTestCase.php

@@ -109,7 +109,7 @@ class Doctrine_ClassTableInheritance_TestCase extends Doctrine_UnitTestCase
         $q = new Doctrine_Query();
         $q->from('CTITest c')->where('c.id = 1');
 
-        $this->assertEqual($q->getSql(), 'SELECT c.id AS c__id, c2.name AS c__name, c2.verified AS c__verified, c3.added AS c__added, c.age AS c__age FROM c_t_i_test_parent4 c LEFT JOIN c_t_i_test_parent2 c2 ON c.id = c2.id LEFT JOIN c_t_i_test_parent3 c3 ON c.id = c3.id WHERE c.id = 1');
+        $this->assertEqual($q->getSql(), 'SELECT c.id AS c__id, c3.added AS c__added, c2.name AS c__name, c2.verified AS c__verified, c.age AS c__age FROM c_t_i_test_parent4 c LEFT JOIN c_t_i_test_parent2 c2 ON c.id = c2.id LEFT JOIN c_t_i_test_parent3 c3 ON c.id = c3.id WHERE c.id = 1');
 
         $record = $q->fetchOne();
         

tests/ColumnAliasTestCase.php

@@ -33,14 +33,60 @@
 class Doctrine_ColumnAlias_TestCase extends Doctrine_UnitTestCase 
 {
     public function prepareData() 
-    { }
+    {
+        $book1 = new Book();
+        $book1->name = 'Das Boot';
+        $book1->save();
+        
+        $record1 = new ColumnAliasTest();
+        $record1->alias1 = 'first';
+        $record1->alias2 = 123;
+        $record1->anotherField = 'camelCase';
+        $record1->bookId = $book1->id;
+        $record1->save();
+        
+        $record2 = new ColumnAliasTest();
+        $record2->alias1 = 'one';
+        $record2->alias2 = 456;
+        $record2->anotherField = 'KoQ';
+        $record2->save();
+        
+        $record2->anotherField = 'foo';
+    }
+    
     public function prepareTables()
     { 
-        $this->tables = array('ColumnAliasTest');
+        $this->tables = array('ColumnAliasTest', 'Book');
         
         parent::prepareTables();
     }
 
+    public function testAliasesAreSupportedForJoins()
+    {
+        $q = new Doctrine_Query();
+        $q->select('c.*, b.name')->from('ColumnAliasTest c')
+                ->innerJoin('c.book b')
+                ->where('c.anotherField = ?', 'camelCase');
+        $result = $q->execute();
+        $this->assertTrue(isset($result[0]->book));
+        $this->assertEqual($result[0]->book->name, 'Das Boot');
+    }
+    
+    public function testAliasesAreSupportedForArrayFetching()
+    {
+        $q = new Doctrine_Query();
+        $q->select('c.*, b.name')->from('ColumnAliasTest c')
+                ->innerJoin('c.book b')
+                ->where('c.anotherField = ?', 'camelCase')
+                ->setHydrationMode(Doctrine::HYDRATE_ARRAY);
+        $result = $q->execute();
+        $this->assertEqual($result[0]['alias1'], 'first');
+        $this->assertEqual($result[0]['alias2'], 123);
+        $this->assertEqual($result[0]['anotherField'], 'camelCase');
+        $this->assertTrue(isset($result[0]['book']));
+        $this->assertEqual($result[0]['book']['name'], 'Das Boot');
+    }
+
     public function testAliasesAreSupportedForRecordPropertyAccessors()
     {
         $record = new ColumnAliasTest;
@@ -50,58 +96,60 @@ class Doctrine_ColumnAlias_TestCase extends Doctrine_UnitTestCase
             
             $this->assertEqual($record->alias1, 'someone');
             $this->assertEqual($record->alias2, 187);
-        } catch(Doctrine_Record_Exception $e) {
+        } catch (Doctrine_Record_Exception $e) {
             $this->fail();
         }
-        $record->save();
     }
+    
     public function testAliasesAreSupportedForDqlSelectPart()
     {
         $q = new Doctrine_Query();
-
-        $q->select('c.alias1, c.alias2')->from('ColumnAliasTest c');
-
+        $q->select('c.alias1, c.alias2, c.anotherField')->from('ColumnAliasTest c');
         $coll = $q->execute();
 
-        $this->assertEqual($coll[0]->alias1, 'someone');
-        $this->assertEqual($coll[0]->alias2, 187);
+        $this->assertEqual($coll[0]->alias1, 'first');
+        $this->assertEqual($coll[0]->alias2, 123);
+        $this->assertEqual($coll[0]->anotherField, 'camelCase');
     }
+    
     public function testAliasesAreSupportedForDqlWherePart()
     {
         $q = new Doctrine_Query();
 
-        $q->select('c.alias1, c.alias2')
+        $q->select('c.alias1, c.alias2, c.anotherField')
           ->from('ColumnAliasTest c')
-          ->where('c.alias1 = ?');
+          ->where('c.anotherField = ?');
 
-        $coll = $q->execute(array('someone'));
+        $coll = $q->execute(array('KoQ'));
 
-        $this->assertEqual($coll[0]->alias1, 'someone');
-        $this->assertEqual($coll[0]->alias2, 187);
+        $this->assertEqual($coll[0]->alias1, 'one');
+        $this->assertEqual($coll[0]->alias2, 456);
+        $this->assertEqual($coll[0]->anotherField, 'KoQ');
     }
+    
     public function testAliasesAreSupportedForDqlAggregateFunctions()
     {
         $q = new Doctrine_Query();
 
-        $q->select('MAX(c.alias1)')
-          ->from('ColumnAliasTest c');
+        $q->select('MAX(c.alias2)')->from('ColumnAliasTest c');
 
         $coll = $q->execute();
 
-        $this->assertEqual($coll[0]->max, 'someone');
+        $this->assertEqual($coll[0]->MAX, 456);
     }
+    
     public function testAliasesAreSupportedForDqlHavingPart()
     {
         $q = new Doctrine_Query();
 
-        $q->select('c.alias1')
+        $q->select('c.alias2')
           ->from('ColumnAliasTest c')
-          ->having('MAX(c.alias2) = 187')
-          ->groupby('c.id');
+          ->groupby('c.id')
+          ->having('c.alias2 > 123');
 
         $coll = $q->execute();
         
-        $this->assertEqual($coll[0]->alias1, 'someone');
+        $this->assertEqual($coll[0]->alias2, 456);
     }
 }
 

tests/Export/RecordTestCase.php

@@ -100,8 +100,8 @@ class Doctrine_Export_Record_TestCase extends Doctrine_UnitTestCase
 
         $this->assertEqual($this->adapter->pop(), 'COMMIT');
         $this->assertEqual($this->adapter->pop(), 'ALTER TABLE cms__category_languages ADD FOREIGN KEY (category_id) REFERENCES cms__category(id) ON DELETE CASCADE');
-        $this->assertEqual($this->adapter->pop(), 'CREATE TABLE cms__category (id BIGINT AUTO_INCREMENT, created DATETIME, parent BIGINT, position MEDIUMINT, active BIGINT, INDEX index_parent_idx (parent), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = INNODB');
         $this->assertEqual($this->adapter->pop(), 'CREATE TABLE cms__category_languages (id BIGINT AUTO_INCREMENT, name TEXT, category_id BIGINT, language_id BIGINT, INDEX index_category_idx (category_id), INDEX index_language_idx (language_id), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = INNODB');
+        $this->assertEqual($this->adapter->pop(), 'CREATE TABLE cms__category (id BIGINT AUTO_INCREMENT, created DATETIME, parent BIGINT, position MEDIUMINT, active BIGINT, INDEX index_parent_idx (parent), PRIMARY KEY(id)) DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci ENGINE = INNODB');    
         $this->assertEqual($this->adapter->pop(), 'BEGIN TRANSACTION');
     }
 

tests/run.php

@@ -280,3 +280,4 @@ $data->addTestCase(new Doctrine_Data_Export_TestCase());
 $test->addTestCase($data);
 
 $test->run();
+