Merge pull request #6939 from open-sausages/pulls/4.0/query-join-alias

API Ensure that all DataQuery joins are aliased based on relationship name

src/ORM/DataList.php

@@ -543,11 +543,13 @@ class DataList extends ViewableData implements SS_List, Filterable, Sortable, Li
                 $relations = explode('.', $field);
                 $fieldName = array_pop($relations);
 
-                // Apply
+                // Apply relation
                 $relationModelName = $query->applyRelation($relations, $linearOnly);
+                $relationPrefix = $query->applyRelationPrefix($relations);
 
                 // Find the db field the relation belongs to
-                $columnName = DataObject::getSchema()->sqlColumnForField($relationModelName, $fieldName);
+                $columnName = DataObject::getSchema()
+                    ->sqlColumnForField($relationModelName, $fieldName, $relationPrefix);
             }
         );
     }

src/ORM/DataObjectSchema.php

@@ -88,15 +88,16 @@ class DataObjectSchema
      *
      * @param string $class Class name (not a table).
      * @param string $field Name of field that belongs to this class (or a parent class)
+     * @param string $tablePrefix Optional prefix for table (alias)
      * @return string The SQL identifier string for the corresponding column for this field
      */
-    public function sqlColumnForField($class, $field)
+    public function sqlColumnForField($class, $field, $tablePrefix = null)
     {
         $table = $this->tableForField($class, $field);
         if (!$table) {
             throw new InvalidArgumentException("\"{$field}\" is not a field on class \"{$class}\"");
         }
-        return "\"{$table}\".\"{$field}\"";
+        return "\"{$tablePrefix}{$table}\".\"{$field}\"";
     }
 
     /**

src/ORM/DataQuery.php

@@ -756,11 +756,38 @@ class DataQuery
         return $this;
     }
 
+    /**
+     * Prefix of all joined table aliases. E.g. ->filter('Banner.Image.Title)'
+     * Will join the Banner, and then Image relations
+     * `$relationPrefx` will be `banner_image_`
+     * Each table in the Image chain will be suffixed to this prefix. E.g.
+     * `banner_image_File` and `banner_image_Image`
+     *
+     * This will be null if no relation is joined.
+     * E.g. `->filter('Title')`
+     *
+     * @param string|array $relation Relation in '.' delimited string, or array of parts
+     * @return string Table prefix
+     */
+    public static function applyRelationPrefix($relation)
+    {
+        if (!$relation) {
+            return null;
+        }
+        if (is_string($relation)) {
+            $relation = explode(".", $relation);
+        }
+        return strtolower(implode('_', $relation)) . '_';
+    }
+
     /**
      * Traverse the relationship fields, and add the table
      * mappings to the query object state. This has to be called
      * in any overloaded {@link SearchFilter->apply()} methods manually.
      *
+     * Note, that in order to filter against the joined relation user code must
+     * use {@see tablePrefix()} to get the table alias used for this relation.
+     *
      * @param string|array $relation The array/dot-syntax relation to follow
      * @param bool $linearOnly Set to true to restrict to linear relations only. Set this
      * if this relation will be used for sorting, and should not include duplicate rows.
@@ -780,20 +807,43 @@ class DataQuery
         $modelClass = $this->dataClass;
 
         $schema = DataObject::getSchema();
+        $currentRelation = [];
         foreach ($relation as $rel) {
+            // Get prefix for join for this table (and parent to join on)
+            $parentPrefix = $this->applyRelationPrefix($currentRelation);
+            $currentRelation[] = $rel;
+            $tablePrefix = $this->applyRelationPrefix($currentRelation);
+
+            // Check has_one
             if ($component = $schema->hasOneComponent($modelClass, $rel)) {
                 // Join via has_one
-                $this->joinHasOneRelation($modelClass, $rel, $component);
+                $this->joinHasOneRelation($modelClass, $rel, $component, $parentPrefix, $tablePrefix);
                 $modelClass = $component;
-            } elseif ($component = $schema->hasManyComponent($modelClass, $rel)) {
+                continue;
+            }
+
+            // Check has_many
+            if ($component = $schema->hasManyComponent($modelClass, $rel)) {
                 // Fail on non-linear relations
                 if ($linearOnly) {
                     throw new InvalidArgumentException("$rel is not a linear relation on model $modelClass");
                 }
                 // Join via has_many
-                $this->joinHasManyRelation($modelClass, $rel, $component);
+                $this->joinHasManyRelation($modelClass, $rel, $component, $parentPrefix, $tablePrefix, 'has_many');
                 $modelClass = $component;
-            } elseif ($component = $schema->manyManyComponent($modelClass, $rel)) {
+                continue;
+            }
+
+            // check belongs_to (like has_many but linear safe)
+            if ($component = $schema->belongsToComponent($modelClass, $rel)) {
+                // Piggy back off has_many logic
+                $this->joinHasManyRelation($modelClass, $rel, $component, $parentPrefix, $tablePrefix, 'belongs_to');
+                $modelClass = $component;
+                continue;
+            }
+
+            // Check many_many
+            if ($component = $schema->manyManyComponent($modelClass, $rel)) {
                 // Fail on non-linear relations
                 if ($linearOnly) {
                     throw new InvalidArgumentException("$rel is not a linear relation on model $modelClass");
@@ -804,31 +854,113 @@ class DataQuery
                     $component['childClass'],
                     $component['parentField'],
                     $component['childField'],
-                    $component['join']
+                    $component['join'],
+                    $parentPrefix,
+                    $tablePrefix
                 );
                 $modelClass = $component['childClass'];
-            } else {
+                continue;
-                throw new InvalidArgumentException("$rel is not a relation on model $modelClass");
             }
+
+            // no relation
+            throw new InvalidArgumentException("$rel is not a relation on model $modelClass");
         }
 
         return $modelClass;
     }
 
+    /**
+     * Join the given has_many relation to this query.
+     * Also works with belongs_to
+     *
+     * Doesn't work with polymorphic relationships
+     *
+     * @param string $localClass Name of class that has the has_many to the joined class
+     * @param string $localField Name of the has_many relationship to join
+     * @param string $foreignClass Class to join
+     * @param string $localPrefix Table prefix for parent class
+     * @param string $foreignPrefix Table prefix to use
+     * @param string $type 'has_many' or 'belongs_to'
+     */
+    protected function joinHasManyRelation(
+        $localClass,
+        $localField,
+        $foreignClass,
+        $localPrefix = null,
+        $foreignPrefix = null,
+        $type = 'has_many'
+    ) {
+        if (!$foreignClass || $foreignClass === DataObject::class) {
+            throw new InvalidArgumentException("Could not find a has_many relationship {$localField} on {$localClass}");
+        }
+        $schema = DataObject::getSchema();
+
+        // Skip if already joined
+        // Note: don't just check base class, since we need to join on the table with the actual relation key
+        $foreignTable = $schema->tableName($foreignClass);
+        $foreignTableAliased = $foreignPrefix . $foreignTable;
+        if ($this->query->isJoinedTo($foreignTableAliased)) {
+            return;
+        }
+
+        // Join table with associated has_one
+        /** @var DataObject $model */
+        $foreignKey = $schema->getRemoteJoinField($localClass, $localField, $type, $polymorphic);
+        $localIDColumn = $schema->sqlColumnForField($localClass, 'ID', $localPrefix);
+        if ($polymorphic) {
+            $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}ID", $foreignPrefix);
+            $foreignKeyClassColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}Class", $foreignPrefix);
+            $localClassColumn = $schema->sqlColumnForField($localClass, 'ClassName', $localPrefix);
+            $joinExpression =
+                "{$foreignKeyIDColumn} = {$localIDColumn} AND {$foreignKeyClassColumn} = {$localClassColumn}";
+        } else {
+            $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, $foreignKey, $foreignPrefix);
+            $joinExpression = "{$foreignKeyIDColumn} = {$localIDColumn}";
+        }
+        $this->query->addLeftJoin(
+            $foreignTable,
+            $joinExpression,
+            $foreignTableAliased
+        );
+
+        // Add join clause to the component's ancestry classes so that the search filter could search on
+        // its ancestor fields.
+        $ancestry = ClassInfo::ancestry($foreignClass, true);
+        $ancestry = array_reverse($ancestry);
+        foreach ($ancestry as $ancestor) {
+            $ancestorTable = $schema->tableName($ancestor);
+            if ($ancestorTable !== $foreignTable) {
+                $ancestorTableAliased = $foreignPrefix.$ancestorTable;
+                $this->query->addLeftJoin(
+                    $ancestorTable,
+                    "\"{$foreignTableAliased}\".\"ID\" = \"{$ancestorTableAliased}\".\"ID\"",
+                    $ancestorTableAliased
+                );
+            }
+        }
+    }
+
     /**
      * Join the given class to this query with the given key
      *
      * @param string $localClass Name of class that has the has_one to the joined class
      * @param string $localField Name of the has_one relationship to joi
      * @param string $foreignClass Class to join
+     * @param string $localPrefix Table prefix to use for local class
+     * @param string $foreignPrefix Table prefix to use for joined table
      */
-    protected function joinHasOneRelation($localClass, $localField, $foreignClass)
+    protected function joinHasOneRelation(
-    {
+        $localClass,
+        $localField,
+        $foreignClass,
+        $localPrefix = null,
+        $foreignPrefix = null
+    ) {
         if (!$foreignClass) {
             throw new InvalidArgumentException("Could not find a has_one relationship {$localField} on {$localClass}");
         }
 
-        if ($foreignClass === 'SilverStripe\ORM\DataObject') {
+        if ($foreignClass === DataObject::class) {
             throw new InvalidArgumentException(
                 "Could not join polymorphic has_one relationship {$localField} on {$localClass}"
             );
@@ -838,14 +970,18 @@ class DataQuery
         // Skip if already joined
         $foreignBaseClass = $schema->baseDataClass($foreignClass);
         $foreignBaseTable = $schema->tableName($foreignBaseClass);
-        if ($this->query->isJoinedTo($foreignBaseTable)) {
+        if ($this->query->isJoinedTo($foreignPrefix.$foreignBaseTable)) {
             return;
         }
 
         // Join base table
-        $foreignIDColumn = $schema->sqlColumnForField($foreignBaseClass, 'ID');
+        $foreignIDColumn = $schema->sqlColumnForField($foreignBaseClass, 'ID', $foreignPrefix);
-        $localColumn = $schema->sqlColumnForField($localClass, "{$localField}ID");
+        $localColumn = $schema->sqlColumnForField($localClass, "{$localField}ID", $localPrefix);
-        $this->query->addLeftJoin($foreignBaseTable, "{$foreignIDColumn} = {$localColumn}");
+        $this->query->addLeftJoin(
+            $foreignBaseTable,
+            "{$foreignIDColumn} = {$localColumn}",
+            $foreignPrefix.$foreignBaseTable
+        );
 
         // Add join clause to the component's ancestry classes so that the search filter could search on
         // its ancestor fields.
@@ -855,63 +991,17 @@ class DataQuery
             foreach ($ancestry as $ancestor) {
                 $ancestorTable = $schema->tableName($ancestor);
                 if ($ancestorTable !== $foreignBaseTable) {
-                    $this->query->addLeftJoin($ancestorTable, "{$foreignIDColumn} = \"{$ancestorTable}\".\"ID\"");
+                    $ancestorTableAliased = $foreignPrefix.$ancestorTable;
+                    $this->query->addLeftJoin(
+                        $ancestorTable,
+                        "{$foreignIDColumn} = \"{$ancestorTableAliased}\".\"ID\"",
+                        $ancestorTableAliased
+                    );
                 }
             }
         }
     }
 
-    /**
-     * Join the given has_many relation to this query.
-     *
-     * Doesn't work with polymorphic relationships
-     *
-     * @param string $localClass Name of class that has the has_many to the joined class
-     * @param string $localField Name of the has_many relationship to join
-     * @param string $foreignClass Class to join
-     */
-    protected function joinHasManyRelation($localClass, $localField, $foreignClass)
-    {
-        if (!$foreignClass || $foreignClass === 'SilverStripe\ORM\DataObject') {
-            throw new InvalidArgumentException("Could not find a has_many relationship {$localField} on {$localClass}");
-        }
-        $schema = DataObject::getSchema();
-
-        // Skip if already joined
-        $foreignTable = $schema->tableName($foreignClass);
-        if ($this->query->isJoinedTo($foreignTable)) {
-            return;
-        }
-
-        // Join table with associated has_one
-        /** @var DataObject $model */
-        $foreignKey = $schema->getRemoteJoinField($localClass, $localField, 'has_many', $polymorphic);
-        $localIDColumn = $schema->sqlColumnForField($localClass, 'ID');
-        if ($polymorphic) {
-            $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}ID");
-            $foreignKeyClassColumn = $schema->sqlColumnForField($foreignClass, "{$foreignKey}Class");
-            $localClassColumn = $schema->sqlColumnForField($localClass, 'ClassName');
-            $this->query->addLeftJoin(
-                $foreignTable,
-                "{$foreignKeyIDColumn} = {$localIDColumn} AND {$foreignKeyClassColumn} = {$localClassColumn}"
-            );
-        } else {
-            $foreignKeyIDColumn = $schema->sqlColumnForField($foreignClass, $foreignKey);
-            $this->query->addLeftJoin($foreignTable, "{$foreignKeyIDColumn} = {$localIDColumn}");
-        }
-
-        // Add join clause to the component's ancestry classes so that the search filter could search on
-        // its ancestor fields.
-        $ancestry = ClassInfo::ancestry($foreignClass, true);
-        $ancestry = array_reverse($ancestry);
-        foreach ($ancestry as $ancestor) {
-            $ancestorTable = $schema->tableName($ancestor);
-            if ($ancestorTable !== $foreignTable) {
-                $this->query->addInnerJoin($ancestorTable, "\"{$foreignTable}\".\"ID\" = \"{$ancestorTable}\".\"ID\"");
-            }
-        }
-    }
-
     /**
      * Join table via many_many relationship
      *
@@ -921,6 +1011,8 @@ class DataQuery
      * @param string $parentField
      * @param string $componentField
      * @param string $relationClassOrTable Name of relation table
+     * @param string $parentPrefix Table prefix for parent class
+     * @param string $componentPrefix Table prefix to use for both joined and mapping table
      */
     protected function joinManyManyRelationship(
         $relationClass,
@@ -928,7 +1020,9 @@ class DataQuery
         $componentClass,
         $parentField,
         $componentField,
-        $relationClassOrTable
+        $relationClassOrTable,
+        $parentPrefix = null,
+        $componentPrefix = null
     ) {
         $schema = DataObject::getSchema();
 
@@ -936,23 +1030,30 @@ class DataQuery
             $relationClassOrTable = $schema->tableName($relationClassOrTable);
         }
 
-        // Join on parent table
+        // Check if already joined to component alias (skip join table for the check)
-        $parentIDColumn = $schema->sqlColumnForField($parentClass, 'ID');
+        $componentBaseClass = $schema->baseDataClass($componentClass);
+        $componentBaseTable = $schema->tableName($componentBaseClass);
+        $componentAliasedTable = $componentPrefix . $componentBaseTable;
+        if ($this->query->isJoinedTo($componentAliasedTable)) {
+            return;
+        }
+
+        // Join parent class to join table
+        $relationAliasedTable = $componentPrefix.$relationClassOrTable;
+        $parentIDColumn = $schema->sqlColumnForField($parentClass, 'ID', $parentPrefix);
         $this->query->addLeftJoin(
             $relationClassOrTable,
-            "\"$relationClassOrTable\".\"$parentField\" = {$parentIDColumn}"
+            "\"{$relationAliasedTable}\".\"{$parentField}\" = {$parentIDColumn}",
+            $relationAliasedTable
         );
 
         // Join on base table of component class
-        $componentBaseClass = $schema->baseDataClass($componentClass);
+        $componentIDColumn = $schema->sqlColumnForField($componentBaseClass, 'ID', $componentPrefix);
-        $componentBaseTable = $schema->tableName($componentBaseClass);
-        $componentIDColumn = $schema->sqlColumnForField($componentBaseClass, 'ID');
-        if (!$this->query->isJoinedTo($componentBaseTable)) {
             $this->query->addLeftJoin(
                 $componentBaseTable,
-                "\"$relationClassOrTable\".\"$componentField\" = {$componentIDColumn}"
+                "\"{$relationAliasedTable}\".\"{$componentField}\" = {$componentIDColumn}",
+                $componentAliasedTable
             );
-        }
 
         // Add join clause to the component's ancestry classes so that the search filter could search on
         // its ancestor fields.
@@ -960,8 +1061,13 @@ class DataQuery
         $ancestry = array_reverse($ancestry);
         foreach ($ancestry as $ancestor) {
             $ancestorTable = $schema->tableName($ancestor);
-            if ($ancestorTable != $componentBaseTable && !$this->query->isJoinedTo($ancestorTable)) {
+            if ($ancestorTable !== $componentBaseTable) {
-                $this->query->addLeftJoin($ancestorTable, "{$componentIDColumn} = \"{$ancestorTable}\".\"ID\"");
+                $ancestorTableAliased = $componentPrefix.$ancestorTable;
+                $this->query->addLeftJoin(
+                    $ancestorTable,
+                    "{$componentIDColumn} = \"{$ancestorTableAliased}\".\"ID\"",
+                    $ancestorTableAliased
+                );
             }
         }
     }
@@ -1060,7 +1166,6 @@ class DataQuery
 
     /**
      * An arbitrary store of query parameters that can be used by decorators.
-     * @todo This will probably be made obsolete if we have subclasses of DataList and/or DataQuery.
      */
     private $queryParams;
 

src/ORM/Filters/FulltextFilter.php

@@ -91,9 +91,11 @@ class FulltextFilter extends SearchFilter
      */
     protected function prepareColumns($columns)
     {
+        $prefix = DataQuery::applyRelationPrefix($this->relation);
         $table = DataObject::getSchema()->tableForField($this->model, current($columns));
-        $columns = array_map(function ($column) use ($table) {
+        $fullTable = $prefix . $table;
-            return Convert::symbol2sql("$table.$column");
+        $columns = array_map(function ($col) use ($fullTable) {
+            return "\"{$fullTable}\".\"{$col}\"";
         }, $columns);
         return implode(',', $columns);
     }

src/ORM/Filters/SearchFilter.php

@@ -2,6 +2,7 @@
 
 namespace SilverStripe\ORM\Filters;
 
+use SilverStripe\Core\ClassInfo;
 use SilverStripe\Core\Injector\Injectable;
 use SilverStripe\ORM\DataObject;
 use SilverStripe\ORM\DataQuery;
@@ -28,7 +29,11 @@ abstract class SearchFilter
     use Injectable;
 
     /**
-     * @var string Classname of the inspected {@link DataObject}
+     * Classname of the inspected {@link DataObject}.
+     * If pointing to a relation, this will be the classname of the leaf
+     * class in the relation
+     *
+     * @var string
      */
     protected $model;
 
@@ -53,11 +58,13 @@ abstract class SearchFilter
     protected $modifiers;
 
     /**
-     * @var string Name of a has-one, has-many or many-many relation (not the classname).
+     * @var array Parts of a has-one, has-many or many-many relation (not the classname).
      * Set in the constructor as part of the name in dot-notation, and used in
      * {@link applyRelation()}.
+     *
+     * Also used to build table prefix (see getRelationTablePrefix)
      */
-    protected $relation;
+    protected $relation = [];
 
     /**
      * An array of data about an aggregate column being used
@@ -137,11 +144,11 @@ abstract class SearchFilter
      * Set the root model class to be selected by this
      * search query.
      *
-     * @param string $className
+     * @param string|DataObject $className
      */
     public function setModel($className)
     {
-        $this->model = $className;
+        $this->model = ClassInfo::class_name($className);
     }
 
     /**
@@ -261,6 +268,7 @@ abstract class SearchFilter
                 "Model supplied to " . static::class . " should be an instance of DataObject."
             );
         }
+        $tablePrefix = DataQuery::applyRelationPrefix($this->relation);
         $schema = DataObject::getSchema();
 
         if ($this->aggregate) {
@@ -280,24 +288,25 @@ abstract class SearchFilter
                 ));
             }
             return sprintf(
-                '%s("%s".%s)',
+                '%s("%s%s".%s)',
                 $function,
+                $tablePrefix,
                 $table,
                 $column ? "\"$column\"" : '"ID"'
             );
         }
 
 
-        // Find table this field belongs to
+        // Check if this column is a table on the current model
         $table = $schema->tableForField($this->model, $this->name);
-        if (!$table) {
+        if ($table) {
-            // fallback to the provided name in the event of a joined column
+            return $schema->sqlColumnForField($this->model, $this->name, $tablePrefix);
-            // name (as the candidate class doesn't check joined records)
-            $parts = explode('.', $this->fullName);
-            return '"' . implode('"."', $parts) . '"';
         }
 
-        return sprintf('"%s"."%s"', $table, $this->name);
+        // fallback to the provided name in the event of a joined column
+        // name (as the candidate class doesn't check joined records)
+        $parts = explode('.', $this->fullName);
+        return '"' . implode('"."', $parts) . '"';
     }
 
     /**

tests/php/Forms/GridField/GridFieldSortableHeaderTest.php

@@ -136,12 +136,12 @@ class GridFieldSortableHeaderTest extends SapphireTest
         $config = new GridFieldConfig_RecordEditor();
         $gridField = new GridField('testfield', 'testfield', $list, $config);
         $state = $gridField->State->GridFieldSortableHeader;
-        $compontent = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class);
+        $component = $gridField->getConfig()->getComponentByType(GridFieldSortableHeader::class);
 
         // Test that inherited dataobjects will work correctly
         $state->SortColumn = 'Cheerleader.Hat.Colour';
         $state->SortDirection = 'asc';
-        $relationListA = $compontent->getManipulatedData($gridField, $list);
+        $relationListA = $component->getManipulatedData($gridField, $list);
         $relationListAsql = Convert::nl2os($relationListA->sql(), ' ');
 
         // Assert that all tables are joined properly
@@ -153,12 +153,16 @@ class GridFieldSortableHeaderTest extends SapphireTest
         );
         $this->assertContains(
             'LEFT JOIN "GridFieldSortableHeaderTest_Cheerleader" '
-            . 'ON "GridFieldSortableHeaderTest_Cheerleader"."ID" = "GridFieldSortableHeaderTest_Team"."CheerleaderID"',
+            . 'AS "cheerleader_GridFieldSortableHeaderTest_Cheerleader" '
+            . 'ON "cheerleader_GridFieldSortableHeaderTest_Cheerleader"."ID" = '
+            . '"GridFieldSortableHeaderTest_Team"."CheerleaderID"',
             $relationListAsql
         );
         $this->assertContains(
             'LEFT JOIN "GridFieldSortableHeaderTest_CheerleaderHat" '
-            . 'ON "GridFieldSortableHeaderTest_CheerleaderHat"."ID" = "GridFieldSortableHeaderTest_Cheerleader"."HatID"',
+            . 'AS "cheerleader_hat_GridFieldSortableHeaderTest_CheerleaderHat" '
+            . 'ON "cheerleader_hat_GridFieldSortableHeaderTest_CheerleaderHat"."ID" = '
+            . '"cheerleader_GridFieldSortableHeaderTest_Cheerleader"."HatID"',
             $relationListAsql
         );
 
@@ -168,7 +172,7 @@ class GridFieldSortableHeaderTest extends SapphireTest
             $relationListA->column('City')
         );
         $state->SortDirection = 'desc';
-        $relationListAdesc = $compontent->getManipulatedData($gridField, $list);
+        $relationListAdesc = $component->getManipulatedData($gridField, $list);
         $this->assertEquals(
             array('Melbourne', 'Wellington', 'Auckland', 'Cologne'),
             $relationListAdesc->column('City')
@@ -177,7 +181,7 @@ class GridFieldSortableHeaderTest extends SapphireTest
         // Test subclasses of tables
         $state->SortColumn = 'CheerleadersMom.Hat.Colour';
         $state->SortDirection = 'asc';
-        $relationListB = $compontent->getManipulatedData($gridField, $list);
+        $relationListB = $component->getManipulatedData($gridField, $list);
         $relationListBsql = $relationListB->sql();
 
         // Assert that subclasses are included in the query
@@ -188,20 +192,27 @@ class GridFieldSortableHeaderTest extends SapphireTest
             $relationListBsql
         );
         // Joined tables are joined basetable first
+        // Note: CheerLeader is base of Mom table, hence the alias
         $this->assertContains(
             'LEFT JOIN "GridFieldSortableHeaderTest_Cheerleader" '
-                . 'ON "GridFieldSortableHeaderTest_Cheerleader"."ID" = "GridFieldSortableHeaderTest_Team"."CheerleadersMomID"',
+            . 'AS "cheerleadersmom_GridFieldSortableHeaderTest_Cheerleader" '
+            . 'ON "cheerleadersmom_GridFieldSortableHeaderTest_Cheerleader"."ID" = '
+            . '"GridFieldSortableHeaderTest_Team"."CheerleadersMomID"',
             $relationListBsql
         );
         // Then the basetable of the joined record is joined to the specific subtable
         $this->assertContains(
             'LEFT JOIN "GridFieldSortableHeaderTest_Mom" '
-            . 'ON "GridFieldSortableHeaderTest_Cheerleader"."ID" = "GridFieldSortableHeaderTest_Mom"."ID"',
+            . 'AS "cheerleadersmom_GridFieldSortableHeaderTest_Mom" '
+            . 'ON "cheerleadersmom_GridFieldSortableHeaderTest_Cheerleader"."ID" = '
+            . '"cheerleadersmom_GridFieldSortableHeaderTest_Mom"."ID"',
             $relationListBsql
         );
         $this->assertContains(
             'LEFT JOIN "GridFieldSortableHeaderTest_CheerleaderHat" '
-            . 'ON "GridFieldSortableHeaderTest_CheerleaderHat"."ID" = "GridFieldSortableHeaderTest_Cheerleader"."HatID"',
+            . 'AS "cheerleadersmom_hat_GridFieldSortableHeaderTest_CheerleaderHat" '
+            . 'ON "cheerleadersmom_hat_GridFieldSortableHeaderTest_CheerleaderHat"."ID" = '
+            . '"cheerleadersmom_GridFieldSortableHeaderTest_Cheerleader"."HatID"',
             $relationListBsql
         );
 
@@ -212,7 +223,7 @@ class GridFieldSortableHeaderTest extends SapphireTest
             $relationListB->column('City')
         );
         $state->SortDirection = 'desc';
-        $relationListBdesc = $compontent->getManipulatedData($gridField, $list);
+        $relationListBdesc = $component->getManipulatedData($gridField, $list);
         $this->assertEquals(
             array('Melbourne', 'Wellington', 'Auckland', 'Cologne'),
             $relationListBdesc->column('City')

tests/php/ORM/DataListTest.php

@@ -5,10 +5,12 @@ namespace SilverStripe\ORM\Tests;
 use SilverStripe\Core\Convert;
 use SilverStripe\Core\Injector\InjectorNotFoundException;
 use SilverStripe\ORM\DataList;
+use SilverStripe\ORM\DataQuery;
 use SilverStripe\ORM\DB;
 use SilverStripe\ORM\Filterable;
 use SilverStripe\ORM\Filters\ExactMatchFilter;
 use SilverStripe\Dev\SapphireTest;
+use SilverStripe\ORM\Tests\DataObjectTest\Bracket;
 use SilverStripe\ORM\Tests\DataObjectTest\EquipmentCompany;
 use SilverStripe\ORM\Tests\DataObjectTest\Fan;
 use SilverStripe\ORM\Tests\DataObjectTest\Player;
@@ -851,6 +853,69 @@ class DataListTest extends SapphireTest
         $this->assertEquals(2, $gtList->count());
     }
 
+    /**
+     * Test that a filter correctly aliases relationships that share common classes
+     */
+    public function testFilterSharedRelationalClasses()
+    {
+        /** @var Bracket $final1 */
+        $final1 = $this->objFromFixture(Bracket::class, 'final');
+        $prefinal1 = $this->objFromFixture(Bracket::class, 'prefinal1');
+        $prefinal2 = $this->objFromFixture(Bracket::class, 'prefinal2');
+        $semifinal1 = $this->objFromFixture(Bracket::class, 'semifinal1');
+        $team2 = $this->objFromFixture(Team::class, 'team2');
+
+        // grand child can be found from parent
+        $found = Bracket::get()->filter('Next.Next.Title', $final1->Title);
+        $this->assertDOSEquals(
+            [['Title' => $semifinal1->Title]],
+            $found
+        );
+
+        // grand child can be found from child
+        $found = Bracket::get()->filter('Next.Title', $prefinal1->Title);
+        $this->assertDOSEquals(
+            [['Title' => $semifinal1->Title]],
+            $found
+        );
+
+        // child can be found from parent
+        $found = Bracket::get()->filter('Next.Title', $final1->Title);
+        $this->assertDOSEquals(
+            [
+                ['Title' => $prefinal1->Title],
+                ['Title' => $prefinal2->Title]
+            ],
+            $found
+        );
+
+        // Complex filter, get brackets where the following bracket was won by team 1
+        // Note: Includes results from multiple levels
+        $found = Bracket::get()->filter('Next.Winner.Title', $team2->Title);
+        $this->assertDOSEquals(
+            [
+                ['Title' => $prefinal1->Title],
+                ['Title' => $prefinal2->Title],
+                ['Title' => $semifinal1->Title]
+            ],
+            $found
+        );
+    }
+
+    public function testFilterOnImplicitJoinWithSharedInheritance()
+    {
+        $list = DataObjectTest\RelationChildFirst::get()->filter(array(
+            'ManyNext.ID' => array(
+                $this->idFromFixture(DataObjectTest\RelationChildSecond::class, 'test1'),
+                $this->idFromFixture(DataObjectTest\RelationChildSecond::class, 'test2'),
+            ),
+        ));
+        $this->assertEquals(2, $list->count());
+        $ids = $list->column('ID');
+        $this->assertContains($this->idFromFixture(DataObjectTest\RelationChildFirst::class, 'test1'), $ids);
+        $this->assertContains($this->idFromFixture(DataObjectTest\RelationChildFirst::class, 'test2'), $ids);
+    }
+
     public function testFilterAny()
     {
         $list = TeamComment::get();
@@ -1229,13 +1294,13 @@ class DataListTest extends SapphireTest
         $filter = new ExactMatchFilter(
             'Comments.Count()'
         );
-        $filter->setModel(new DataObjectTest\Team());
+        $filter->apply(new DataQuery(DataObjectTest\Team::class));
-        $this->assertEquals('COUNT("DataObjectTest_Team"."ID")', $filter->getDBName());
+        $this->assertEquals('COUNT("comments_DataObjectTest_TeamComment"."ID")', $filter->getDBName());
 
         foreach (['Comments.Max(ID)', 'Comments.Max( ID )', 'Comments.Max(  ID)'] as $name) {
             $filter = new ExactMatchFilter($name);
-            $filter->setModel(new DataObjectTest\Team());
+            $filter->apply(new DataQuery(DataObjectTest\Team::class));
-            $this->assertEquals('MAX("DataObjectTest_Team"."ID")', $filter->getDBName());
+            $this->assertEquals('MAX("comments_DataObjectTest_TeamComment"."ID")', $filter->getDBName());
         }
     }
 

tests/php/ORM/DataObjectDuplicationTest/Class4.php

@@ -13,6 +13,8 @@ use SilverStripe\ORM\ManyManyList;
  */
 class Class4 extends DataObject implements TestOnly
 {
+    private static $table_name = 'DataObjectDuplicateTest_Class4';
+
     private static $db = [
         'Title' => 'Varchar',
     ];

tests/php/ORM/DataObjectTest.php

@@ -51,6 +51,10 @@ class DataObjectTest extends SapphireTest
         DataObjectTest\Ploy::class,
         DataObjectTest\Bogey::class,
         DataObjectTest\Sortable::class,
+        DataObjectTest\Bracket::class,
+        DataObjectTest\RelationParent::class,
+        DataObjectTest\RelationChildFirst::class,
+        DataObjectTest\RelationChildSecond::class,
     );
 
     protected static function getExtraDataObjects()

tests/php/ORM/DataObjectTest.yml

@@ -145,3 +145,37 @@ SilverStripe\ORM\Tests\DataObjectTest\Company:
   company1:
     Name: 'Team co.'
     Owner: =>SilverStripe\ORM\Tests\DataObjectTest\Player.player2
+SilverStripe\ORM\Tests\DataObjectTest\Bracket:
+  final:
+    Title: 'Final'
+    Winner: =>SilverStripe\ORM\Tests\DataObjectTest\Team.team2
+  prefinal1:
+    Title: 'Prelim final 1'
+    Next: =>SilverStripe\ORM\Tests\DataObjectTest\Bracket.final
+    Winner: =>SilverStripe\ORM\Tests\DataObjectTest\Team.team2
+  prefinal2:
+    Title: 'Prelim final 2'
+    Next: =>SilverStripe\ORM\Tests\DataObjectTest\Bracket.final
+    Winner: =>SilverStripe\ORM\Tests\DataObjectTest\Team.team1
+  semifinal1:
+    Title: 'Semi final 1'
+    Next: =>SilverStripe\ORM\Tests\DataObjectTest\Bracket.prefinal1
+    Winner: =>SilverStripe\ORM\Tests\DataObjectTest\Team.team2
+SilverStripe\ORM\Tests\DataObjectTest\RelationChildSecond:
+  test1:
+    Title: 'Test 1'
+  test2:
+    Title: 'Test 2'
+  test3:
+    Title: 'Test 3'
+SilverStripe\ORM\Tests\DataObjectTest\RelationChildFirst:
+  test1:
+    Title: 'Test1'
+    ManyNext:
+      - =>SilverStripe\ORM\Tests\DataObjectTest\RelationChildSecond.test1
+      - =>SilverStripe\ORM\Tests\DataObjectTest\RelationChildSecond.test2
+  test2:
+    Title: 'Test2'
+    ManyNext:
+      - =>SilverStripe\ORM\Tests\DataObjectTest\RelationChildSecond.test1
+      - =>SilverStripe\ORM\Tests\DataObjectTest\RelationChildSecond.test3

tests/php/ORM/DataObjectTest/Bracket.php

@@ -0,0 +1,32 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\ORM\DataObject;
+
+/**
+ * @property string $Title
+ * @method Bracket Parent()
+ * @method Team First()
+ * @method Team Second()
+ * @method Team Winner()
+ */
+class Bracket extends DataObject
+{
+    private static $table_name = 'DataObjectTest_Bracket';
+
+    private static $db = [
+        'Title' => 'Varchar(100)',
+    ];
+
+    private static $has_one = [
+        'Next' => Bracket::class,
+        'First' => Team::class,
+        'Second' => Team::class,
+        'Winner' => Team::class,
+    ];
+
+    private static $has_many = [
+        'Previous' => Bracket::class,
+    ];
+}

tests/php/ORM/DataObjectTest/CEO.php

@@ -7,8 +7,8 @@ class CEO extends Staff
     private static $table_name = 'DataObjectTest_CEO';
 
     private static $belongs_to = array(
-        'Company' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Company.CEO',
+        'Company' => Company::class.'.CEO',
-        'PreviousCompany' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Company.PreviousCEO',
+        'PreviousCompany' => Company::class.'.PreviousCEO',
-        'CompanyOwned' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Company.Owner'
+        'CompanyOwned' => Company::class.'.Owner'
     );
 }

tests/php/ORM/DataObjectTest/Company.php

@@ -20,7 +20,7 @@ class Company extends DataObject implements TestOnly
     ];
 
     private static $has_many = array(
-        'CurrentStaff' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Staff.CurrentCompany',
+        'CurrentStaff' => Staff::class.'.CurrentCompany',
-        'PreviousStaff' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Staff.PreviousCompany'
+        'PreviousStaff' => Staff::class.'.PreviousCompany'
     );
 }

tests/php/ORM/DataObjectTest/Player.php

@@ -24,13 +24,13 @@ class Player extends Member implements TestOnly
     );
 
     private static $has_many = array(
-        'Fans' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Fan.Favourite', // Polymorphic - Player fans
+        'Fans' => Fan::class.'.Favourite', // Polymorphic - Player fans
-        'CaptainTeams' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Team.Captain',
+        'CaptainTeams' => Team::class.'.Captain',
-        'FoundingTeams' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Team.Founder'
+        'FoundingTeams' => Team::class.'.Founder'
     );
 
     private static $belongs_to = array(
-        'CompanyOwned' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Company.Owner'
+        'CompanyOwned' => Company::class.'.Owner'
     );
 
     private static $searchable_fields = array(

tests/php/ORM/DataObjectTest/RelationChildFirst.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+class RelationChildFirst extends RelationParent
+{
+    private static $table_name = 'DataObjectTest_RelationChildFirst';
+
+    private static $many_many = [
+        'ManyNext' => RelationChildSecond::class,
+    ];
+}

tests/php/ORM/DataObjectTest/RelationChildSecond.php

@@ -0,0 +1,12 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+class RelationChildSecond extends RelationParent
+{
+    private static $table_name = 'DataObjectTest_RelationChildSecond';
+
+    private static $belongs_many_many = [
+        'ManyPrev' => RelationChildFirst::class,
+    ];
+}

tests/php/ORM/DataObjectTest/RelationParent.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace SilverStripe\ORM\Tests\DataObjectTest;
+
+use SilverStripe\Dev\TestOnly;
+use SilverStripe\ORM\DataObject;
+
+class RelationParent extends DataObject implements TestOnly
+{
+    private static $table_name = 'DataObjectTest_RelationParent';
+
+    private static $db = [
+        'Title' => 'Varchar(255)',
+    ];
+}

tests/php/ORM/DataObjectTest/Team.php

@@ -39,8 +39,8 @@ class Team extends DataObject implements TestOnly
     private static $has_many = array(
         'SubTeams' => SubTeam::class,
         'Comments' => TeamComment::class,
-        'Fans' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Fan.Favourite', // Polymorphic - Team fans
+        'Fans' => Fan::class.'.Favourite', // Polymorphic - Team fans
-        'PlayerFans' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\Player.FavouriteTeam'
+        'PlayerFans' => Player::class.'.FavouriteTeam'
     );
 
     private static $many_many = array(
@@ -54,8 +54,8 @@ class Team extends DataObject implements TestOnly
     );
 
     private static $belongs_many_many = array(
-        'Sponsors' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\EquipmentCompany.SponsoredTeams',
+        'Sponsors' => EquipmentCompany::class.'.SponsoredTeams',
-        'EquipmentSuppliers' => 'SilverStripe\\ORM\\Tests\\DataObjectTest\\EquipmentCompany.EquipmentCustomers'
+        'EquipmentSuppliers' => EquipmentCompany::class.'.EquipmentCustomers'
     );
 
     private static $summary_fields = array(

tests/php/ORM/DataQueryTest.php

@@ -75,13 +75,13 @@ class DataQueryTest extends SapphireTest
         // Test applyRelation with two has_ones pointing to the same class
         $dq = new DataQuery(DataQueryTest\ObjectB::class);
         $dq->applyRelation('TestC');
-        $this->assertTrue($dq->query()->isJoinedTo('DataQueryTest_C'));
+        $this->assertTrue($dq->query()->isJoinedTo('testc_DataQueryTest_C'));
-        $this->assertContains('"DataQueryTest_C"."ID" = "DataQueryTest_B"."TestCID"', $dq->sql());
+        $this->assertContains('"testc_DataQueryTest_C"."ID" = "DataQueryTest_B"."TestCID"', $dq->sql());
 
         $dq = new DataQuery(DataQueryTest\ObjectB::class);
         $dq->applyRelation('TestCTwo');
-        $this->assertTrue($dq->query()->isJoinedTo('DataQueryTest_C'));
+        $this->assertTrue($dq->query()->isJoinedTo('testctwo_DataQueryTest_C'));
-        $this->assertContains('"DataQueryTest_C"."ID" = "DataQueryTest_B"."TestCTwoID"', $dq->sql());
+        $this->assertContains('"testctwo_DataQueryTest_C"."ID" = "DataQueryTest_B"."TestCTwoID"', $dq->sql());
     }
 
     public function testApplyReplationDeepInheretence()
@@ -91,7 +91,7 @@ class DataQueryTest extends SapphireTest
         //apply a relation to a relation from an ancestor class
         $newDQ->applyRelation('TestA');
         $this->assertTrue($newDQ->query()->isJoinedTo('DataQueryTest_C'));
-        $this->assertContains('"DataQueryTest_A"."ID" = "DataQueryTest_C"."TestAID"', $newDQ->sql($params));
+        $this->assertContains('"testa_DataQueryTest_A"."ID" = "DataQueryTest_C"."TestAID"', $newDQ->sql($params));
 
         //test many_many relation
 

tests/php/ORM/ManyManyListTest.php

@@ -346,17 +346,11 @@ class ManyManyListTest extends SapphireTest
 
     public function testFilteringOnPreviouslyJoinedTable()
     {
-
+        /** @var ManyManyListTest\Category $category */
-        /**
- * @var ManyManyListTest\Category $category
-*/
         $category = $this->objFromFixture(ManyManyListTest\Category::class, 'categorya');
 
-        /**
+        /** @var ManyManyList $productsRelatedToProductB */
- * @var ManyManyList $productsRelatedToProductB
+        $productsRelatedToProductB = $category->Products()->filter('RelatedProducts.Title', 'Product A');
-*/
-        $productsRelatedToProductB = $category->Products()->filter('RelatedProducts.Title', 'Product B');
-
         $this->assertEquals(1, $productsRelatedToProductB->count());
     }
 }

tests/php/ORM/ManyManyListTest/Category.php

@@ -4,7 +4,11 @@ namespace SilverStripe\ORM\Tests\ManyManyListTest;
 
 use SilverStripe\Dev\TestOnly;
 use SilverStripe\ORM\DataObject;
+use SilverStripe\ORM\ManyManyList;
 
+/**
+ * @method ManyManyList Products()
+ */
 class Category extends DataObject implements TestOnly
 {
     private static $table_name = 'ManyManyListTest_Category';