Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[6.x] Prevent ambiguous column with table name prefix #31174

Merged
merged 1 commit into from Jan 20, 2020
Merged

Conversation

@link08
Copy link
Contributor

link08 commented Jan 20, 2020

Description

When fetching a multi polymorphic relationships, the query builder is creating the query without inject the table name prefix in the query, throwing a MySQL ambiguous column error.

With this fix QueriesRelationships will automatically add the table name before the column definition preventing this kind of errors.

Test case

class Model_1 extends Model
{
    protected $table = 'model_1';

    /**
     * @return MorphToMany
     */
    public function models()
    {
        return $this->morphedByMany(Model_2::class, 'model', 'model_1_relationships');
    }

    /**
     * Filtered models
     *
     * @return MorphToMany
     */
    public function filteredModels()
    {
        return $this->models()
            ->whereHasMorph('model', Model_3::class, function ($model) {
                $model->where(with(new Model_3)->getTable() .'.model_type', Model_4::class);
            });
    }
}

class Model_2 extends Model
{
    protected $table = 'model_2';

    /**
     * @return MorphTo
     */
    public function model()
    {
        return $this->morphTo();
    }
}

class Model_3 extends Model
{
    protected $table = 'model_3';

    /**
     * @return MorphTo
     */
    public function model()
    {
        return $this->morphTo();
    }
}

class Model_4 extends Model
{
}

Error

Fetching Model_1::filteredModels() the query builder return ambiguous column error.

Illuminate\Database\QueryException : SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'model_type' in where clause is ambiguous 

Returned query

SELECT
    `model_2`.*,
    `model_1_relationships`.`model_1_id` AS `pivot_model_1_id`,
    `model_1_relationships`.`model_id` AS `pivot_model_id`,
    `model_1_relationships`.`model_type` AS `pivot_model_type`,
    `model_1_relationships`.`created_at` AS `pivot_created_at`,
    `model_1_relationships`.`updated_at` AS `pivot_updated_at`
FROM
    `model_2`
    INNER JOIN `model_1_relationships` ON `model_2`.`id` = `model_1_relationships`.`model_id`
WHERE
    `model_1_relationships`.`model_1_id` = 1
    AND `model_1_relationships`.`model_type` = 'App\Models\Model_2'
    and((`model_type` = 'App\Models\Model_3'
        AND EXISTS (
            SELECT
                * FROM `model_3`
            WHERE
                `model_2`.`model_id` = `model_3`.`id`
                AND `model_3`.`model_type` = 'App\Models\Model_4'
                AND `model_3`.`deleted_at` IS NULL)))
    AND `model_2`.`deleted_at` IS NULL
ORDER BY
    `position` ASC, `model_2`.`created_at` DESC

Expected query

SELECT
    `model_2`.*,
    `model_1_relationships`.`model_1_id` AS `pivot_model_1_id`,
    `model_1_relationships`.`model_id` AS `pivot_model_id`,
    `model_1_relationships`.`model_type` AS `pivot_model_type`,
    `model_1_relationships`.`created_at` AS `pivot_created_at`,
    `model_1_relationships`.`updated_at` AS `pivot_updated_at`
FROM
    `model_2`
    INNER JOIN `model_1_relationships` ON `model_2`.`id` = `model_1_relationships`.`model_id`
WHERE
    `model_1_relationships`.`model_1_id` = 1
    AND `model_1_relationships`.`model_type` = 'App\Models\Model_2'
    and((`model_2`.`model_type` = 'App\Models\Model_3' # <= Here the FIX
        AND EXISTS (
            SELECT
                * FROM `model_3`
            WHERE
                `model_2`.`model_id` = `model_3`.`id`
                AND `model_3`.`model_type` = 'App\Models\Model_4'
                AND `model_3`.`deleted_at` IS NULL)))
    AND `model_2`.`deleted_at` IS NULL
ORDER BY
    `position` ASC, `model_2`.`created_at` DESC
@GrahamCampbell GrahamCampbell changed the title [6.X] Prevent ambiguous column with table name prefix [6.x] Prevent ambiguous column with table name prefix Jan 20, 2020
@jmarcher

This comment has been minimized.

Copy link
Contributor

jmarcher commented Jan 20, 2020

This will need some tests, you can use the example you described above.

@@ -222,7 +222,7 @@ public function hasMorph($relation, $types, $operator = '>=', $count = 1, $boole
};
}

$query->where($relation->getMorphType(), '=', (new $type)->getMorphClass())
$query->where($this->query->from.'.'.$relation->getMorphType(), '=', (new $type)->getMorphClass())

This comment has been minimized.

Copy link
@mfn

mfn Jan 20, 2020

Contributor

I believe this needs to be $this->wrapTable($this->query->from), there are other such examples if you search for ->wrapTable()

This comment has been minimized.

Copy link
@link08

link08 Jan 20, 2020

Author Contributor

I used the same syntax used in QueriesRelationships@withCount method row 363. Do you suggest to update also that one in the way to standardize?

@taylorotwell

This comment has been minimized.

Copy link
Member

taylorotwell commented Jan 20, 2020

Do not change any other code, but this should probably use wrapTable.

@taylorotwell taylorotwell merged commit d21dcb6 into laravel:6.x Jan 20, 2020
1 check passed
1 check passed
continuous-integration/styleci/pr The analysis has passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants
You can’t perform that action at this time.