DatabaseTests fails on SQL Server

[lcharette]

I've added SQL Server to the test workflow, and I get stuck on this issue. So I'm sharing in case someone as an idea and because writing things down sometimes helps me figure things out.

1) UserFrosting\Sprinkle\Core\Tests\Integration\Database\DatabaseTests::testBelongsToManyThroughPaginatedWithOrderByAggregateColumn
Illuminate\Database\QueryException: SQLSTATE[HY000]: General error: 20018 Column 'permissions.slug' is invalid in the select list because it is not contained in either an aggregate function or the GROUP BY clause. [20018] (severity 16) [select top 1 [permissions].*, (select count(*) from [roles] inner join [permission_roles] on [roles].[id] = [permission_roles].[role_id] where [permissions].[id] = [permission_roles].[permission_id]) as [roles_count], [permissions].[id] from [permissions] inner join [permission_roles] on [permissions].[id] = [permission_roles].[permission_id] inner join [role_users] on [role_users].[role_id] = [permission_roles].[role_id] where [user_id] = 1 group by [permissions].[id] order by [roles_count] desc] (SQL: select top 1 [permissions].*, (select count(*) from [roles] inner join [permission_roles] on [roles].[id] = [permission_roles].[role_id] where [permissions].[id] = [permission_roles].[permission_id]) as [roles_count], [permissions].[id] from [permissions] inner join [permission_roles] on [permissions].[id] = [permission_roles].[permission_id] inner join [role_users] on [role_users].[role_id] = [permission_roles].[role_id] where [user_id] = 1 group by [permissions].[id] order by [roles_count] desc)

~/userfrosting/sprinkle-core/vendor/illuminate/database/Connection.php:712
~/userfrosting/sprinkle-core/vendor/illuminate/database/Connection.php:672
~/userfrosting/sprinkle-core/vendor/illuminate/database/Connection.php:376
~/userfrosting/sprinkle-core/vendor/illuminate/database/Query/Builder.php:2414
~/userfrosting/sprinkle-core/app/src/Database/Builder.php:115
~/userfrosting/sprinkle-core/vendor/illuminate/database/Eloquent/Builder.php:625
~/userfrosting/sprinkle-core/vendor/illuminate/database/Eloquent/Builder.php:609
~/userfrosting/sprinkle-core/app/src/Database/Relations/Concerns/Unique.php:300
~/userfrosting/sprinkle-core/app/src/Database/Relations/Concerns/Unique.php:340
~/userfrosting/sprinkle-core/app/src/Database/Relations/Concerns/Unique.php:250
~/userfrosting/sprinkle-core/app/tests/Integration/Database/DatabaseTests.php:750

Actual problem is this: https://stackoverflow.com/a/13999903/445757

It doesn't affect MySQL because MySQL is smart enough to understand how to handle that (somehow). And it's a "fake" issue in our case, because they are really the same slug (same id).

The failed test is this one :

sprinkle-core/app/tests/Integration/Database/DatabaseTests.php

Line 743 in 631fc77

$paginatedPermissions = $user->permissions()->withCount('roles')->orderBy('roles_count', 'desc')->take(1)->offset(0);

But it's not the query that cause the error. The error is caused by this query :

sprinkle-core/app/src/Database/Relations/Concerns/Unique.php

Line 299 in 631fc77

$modelIds = $constrainedBuilder->get()->pluck($primaryKeyName)->toArray();

Here's the "beautified" SQL:

SELECT
	top 1 [permissions].*,
	(
		SELECT
			count(*)
		FROM
			[roles]
			INNER JOIN [permission_roles] ON [roles].[id] = [permission_roles].[role_id]
		WHERE
			[permissions].[id] = [permission_roles].[permission_id]) AS [roles_count], [permissions].[id]
	FROM
		[permissions]
		INNER JOIN [permission_roles] ON [permissions].[id] = [permission_roles].[permission_id]
		INNER JOIN [role_users] ON [role_users].[role_id] = [permission_roles].[role_id]
	WHERE
		[user_id] = 1
	GROUP BY
		[permissions].[id]
	ORDER BY
		[roles_count] DESC

Problematic part is this : [permissions].*,. Remove the wildcard, remove the slug from columns.

Failed Solution n° 1

We could replace all existing columns (except the required "id" for the group by):

sprinkle-core/app/src/Database/Relations/Concerns/Unique.php

Line 282 in 631fc77

$subQuery->addSelect($relatedPivotKeyName)

...but this removes roles_count and mess up the sort.

There's no way I know to remove * without removing the roles_count (or other user defined select).

Failed Solution n° 2

The issue is with GROUP BY, so we remove the group by right?

Here's the result without Group By and limit :

role_count	id
3	2
3	2
1	3
1	1

So it does work with Limit 1, but Limit 2 won't work (testBelongsToManyThroughPaginated test will fail in this case highlighting this).

Group could be done on the Collection, but this means Limit and Offset also needs to be done In the Collection. Would make the whole thing less efficient.

Failed Solution n° 3

DISTINCT instead of GROUP BY doesn't help too.

$uniqueIdScope = function (Builder $subQuery) use ($relatedPivotKeyName) {
            $subQuery->addSelect($relatedPivotKeyName)
                ->distinct();
                    //  ->groupBy($relatedPivotKeyName);
        };

It does fix testBelongsToManyThroughPaginatedWithOrderByAggregateColumn, but it doesn't fix testBelongsToManyThroughPaginated:

SELECT
	*
FROM ( SELECT DISTINCT
		[permissions].[id],
		row_number() OVER (ORDER BY (
			SELECT
				0)) AS row_num
	FROM
		[permissions]
		INNER JOIN [permission_roles] ON [permissions].[id] = [permission_roles].[permission_id]
		INNER JOIN [role_users] ON [role_users].[role_id] = [permission_roles].[role_id]
	WHERE
		[user_id] = 1) AS temp_table
WHERE
	row_num BETWEEN 2 AND 3
ORDER BY
	row_num

Which returns ID [2, 2] instead of [2, 3]

Original SQL :

SELECT
	*
FROM (
	SELECT
		[permissions].[id],
		row_number() OVER (ORDER BY (
			SELECT
				0)) AS row_num
	FROM
		[permissions]
		INNER JOIN [permission_roles] ON [permissions].[id] = [permission_roles].[permission_id]
		INNER JOIN [role_users] ON [role_users].[role_id] = [permission_roles].[role_id]
	WHERE
		[user_id] = 1
	GROUP BY
		[permissions].[id]) AS temp_table
WHERE
	row_num BETWEEN 2 AND 3
ORDER BY
	row_num

Other solutions

We could "Cheat" the test and force select only 'id' :

$paginatedPermissions = $user->permissions()
    ->select('id')
    ->withCount('roles')
    ->orderBy('roles_count', 'desc')
    ->take(1)
    ->offset(0);

But slug is not part of the results anymore (and bug is still there).

---

So I'm bit stuck now. SQL Server doesn't support this

---

Linked Branch : https://github.com/userfrosting/sprinkle-core/tree/5.0-mssql
Other references:

• https://stackoverflow.com/questions/4611897/group-by-aggregate-function-confusion-in-sql
• https://stackoverflow.com/questions/966176/select-distinct-on-one-column
• https://stackoverflow.com/questions/13999817/reason-for-column-is-invalid-in-the-select-list-because-it-is-not-contained-in-e

[lcharette]

Ping @alexweissman since it's originally your code 😬

[lcharette]

Might be a solution : https://stackoverflow.com/a/55889981/445757

[cmy82]

I was going to post that suggestion earlier, but had to run to my son soccer game. Using a CTE would likely be the solution, it's a lifesaver when working with Oracle as well.

Though you may be able to get away without using a CTE. I don't have MSSQL installed on my home PC, but if I understand the query correctly, this may work:

SELECT
    permissions.*,
    rolecounts.role_count
FROM [permissions] 
LEFT JOIN (SELECT
               role.id,
               count(*) role_count,
               ROW_NUMBER() OVER (PARTITION BY role.ID ORDER BY role.ID) rn
           FROM [roles] role
           INNER JOIN [permission_roles] pr ON pr.role_id = role.id
           GROUP BY role.id) rolecounts ON rolecounts.ID = permissions.ID

[lcharette]

Hum, role_count is not right.

The result should be:

role_count	id	slug
3	2	uri_spit_acid
1	3	uri_slash
1	1	uri_harvest
0	4	uri_royal_jelly

FYI, you can run SQL Server from Docker : https://hub.docker.com/_/microsoft-mssql-server

[cmy82]

Will try it later, but I just noticed I left off one of your join conditions, try:

SELECT
    permissions.*,
    rolecounts.role_count
FROM [permissions] 
LEFT JOIN (SELECT
               role.id,
               count(*) role_count,
               ROW_NUMBER() OVER (PARTITION BY role.ID ORDER BY role.ID) rn
           FROM [roles] role
           INNER JOIN [permission_roles] pr ON pr.role_id = role.id
           INNER JOIN [role_users] ru ON ru.role_id = pr.role_id
           GROUP BY role.id) rolecounts ON rolecounts.ID = permissions.ID

[lcharette]

Nope.

---

I did found something interesting.

This code :

$user->permissions()
     ->withCount('roles')
     ->orderBy('roles_count', 'desc')
     ->toSql();

Return this SQL:

SELECT
	[permissions].*,
	(
		SELECT
			count(*)
		FROM
			[roles]
			INNER JOIN [permission_roles] ON [roles].[id] = [permission_roles].[role_id]
		WHERE
			[permissions].[id] = [permission_roles].[permission_id]
        ) AS [roles_count]
	FROM
		[permissions]
		INNER JOIN [permission_roles] ON [permissions].[id] = [permission_roles].[permission_id]
		INNER JOIN [role_users] ON [role_users].[role_id] = [permission_roles].[role_id]
	WHERE
		[user_id] = 1
	ORDER BY
		[roles_count] DESC

Which returns this when run on the db :

id	slug	role_count
2	uri_spit_acid	3
2	uri_spit_acid	3
3	uri_slash	1
1	uri_harvest	1

But when I do this :

$user->permissions()
            ->withCount('roles')
            ->orderBy('roles_count', 'desc')
            ->get()
            ->toArray();

Only 3 results are returned in the array, not 4... Laravel probably account for the duplicate somehow...

The error only happens when doing pagination on ->take(1), since we do our custom stuff in UserFrosting\Sprinkle\Core\Database\Relations\Concerns\Unique...

$paginatedPermissions = $user->permissions()
     ->withCount('roles')
     ->orderBy('roles_count', 'desc')
     ->take(1)
     ->offset(0)
     ->get()
     ->toArray();

When ->get() is called, the query is mutated by this code :

// $relatedPivotKeyName = permissions.id
$subQuery->addSelect($relatedPivotKeyName)
         ->groupBy($relatedPivotKeyName);

Producing this (failling) SQL :

SELECT
	[permissions].*,
	(
		SELECT
			count(*)
		FROM
			[roles]
			INNER JOIN [permission_roles] ON [roles].[id] = [permission_roles].[role_id]
		WHERE
			[permissions].[id] = [permission_roles].[permission_id]
	) AS [roles_count],
	[permissions].[id] -- Added above
	FROM
		[permissions]
		INNER JOIN [permission_roles] ON [permissions].[id] = [permission_roles].[permission_id]
		INNER JOIN [role_users] ON [role_users].[role_id] = [permission_roles].[role_id]
	WHERE
		[user_id] = 1
	GROUP BY
		[permissions].[id] -- Added above
	ORDER BY
		[roles_count] DESC

Remove the [permissions].*,, and the expected result is :

role_count	id
3	2
1	3
1	1

[cmy82]

You beat me to the punch. I just noticed I had the other join on the wrong query. But I think I know where I went wrong.
If I run this in MySQL, I get an output similar to what you have above with the duplicate rows:

SELECT
    permissions.*,
    rolecounts.role_count
FROM userfrosting.permissions 
LEFT JOIN (SELECT
               role.id,
               count(*) role_count
               #ROW_NUMBER() OVER (PARTITION BY role.ID ORDER BY role.ID) rn
           FROM roles role
           INNER JOIN permission_roles pr ON pr.role_id = role.id
           GROUP BY role.id) rolecounts ON rolecounts.ID = permissions.ID # and rn = 1
INNER JOIN role_users ru ON ru.role_id = rolecounts.id

Of course there is no row_number in MySQL, but if you do that and add AND rn = 1 on the subquery join, you should get just the one row and not the multiple

[lcharette]

Query 1 ERROR: Msg: 102, Line 10, State: 1, Level: 15
Incorrect syntax near '#ROW_NUMBER'.

---

Problem is, once the correct SQL is found, it will need to be converted to the correct Eloquent code.

Only 3 results are returned in the array, not 4... Laravel probably account for the duplicate somehow...

👀

sprinkle-core/app/src/Database/Relations/Concerns/Unique.php

Lines 354 to 356 in 6df7706

	if ($condenseModels) {
	$models = $this->condenseModels($models);
	}

sprinkle-core/app/src/Database/Relations/Concerns/Unique.php

Line 387 in 6df7706

$models = $this->related->newCollection($models)->unique();

Problem is this removes duplicates after the query, but when you limit the query, it needs to be done BEFORE, otherwise LIMIT 2 will return the same row, which then will be parsed through this and return only one row...

I wonder if in this case, the limit could should be done after the query is run ? Might have performance issue with large query, but since this is all related to belongsToManyThrough... @alexweissman any thought on performance issue?

EDIT:
I guess the performance hit on doing the limit on the whole results is similar than doing two queries? I guess this code is only used in Sprunje used to display the user permissions, if it's used at all ?

[cmy82]

Ha, the # is a comment in MySQL, remove them for MSSQL

[lcharette]

Ha, the # is a comment in MySQL, remove them for MSSQL

SELECT
	permissions.*,
	rolecounts.role_count
FROM
	permissions
	LEFT JOIN (
		SELECT
			role.id,
			count(*) role_count,
			ROW_NUMBER() OVER (PARTITION BY role.ID ORDER BY role.ID) rn
		FROM
			roles ROLE
			INNER JOIN permission_roles pr ON pr.role_id = role.id
		GROUP BY
			role.id) rolecounts ON rolecounts.ID = permissions.ID and rn = 1
	INNER JOIN role_users ru ON ru.role_id = rolecounts.id

Not quite, but closer