Are scalar subqueries in a group by a bad practice?

Question

I have this query. Should the nested select be avoided ? Is there a better way ?

WITH cte(oi, oIdOf) AS (
    SELECT ROW_NUMBER() OVER (ORDER BY resIdOf), resIdOf
    FROM @res
    WHERE resIdOf<>0
    GROUP BY resIdOf    
)
INSERT INTO @fop
SELECT x.xIdOf
        ,x.xIdBe
        ,x.xLgnBe
        ,(SELECT e.BE_Commercial FROM BE_ENLEVEMENT AS e WHERE e.BE_Numero_BE=x.xIdBe)
        ,SUM(x.xCoeff)
FROM cte AS o
CROSS APPLY dbo.ft_grapheOfOrigine(o.oIdOf) AS x
GROUP BY x.xIdOf,x.xIdBe,x.xLgnBe;

Answer 1

I am not a fan of correlated subqueries with aggregation. The aggregation conditions can be tricky to get right, because the correlation conditions need to refer to the values after the aggregation.

In fact, correlated subqueries can be quite useful, but the logic is often implemented using LEFT JOIN anyway.

More importantly, it is really simple to rewrite them. So:

SELECT x.xIdOf, x.xIdBe, x.xLgnBe,
       e.BE_Commercial,
       SUM(x.xCoeff)
FROM cte o CROSS APPLY 
     dbo.ft_grapheOfOrigine(o.oIdOf) AS x LEFT JOIN
     BE_ENLEVEMENT e
     ON e.BE_Numero_BE = x.xIdBe
GROUP BY x.xIdOf, x.xIdBe, x.xLgnBe, e.BE_Commercial;

From a performance perspective, this is not exactly the same as your query, because the JOIN happens before the aggregation and there is an additional aggregation key. However, I think this would be a very minor impact on performance, given that it is already doing an aggregation.

If this is an issue, you can use a subquery to get essentially the same execution plan.

Answer 2

You are using a subquery in the SELECT clause and I find it very appropriate here.

The table BE_ENLEVEMENT serves as a lookup table here, where you look up the one BE_Commercial for each selected xIdBe. If you had used a join instead, I wouldn't have seen that there is only one BE_Commercial per xIdBe.

This means the subquery in the select clause makes the query easier to understand, which is good. Readability enhances maintainability.

Answer 3

Your current query will give bad performance, if there are lot of data inserting.

If it insert few like 10 rows then you can ignore it.

Scalar UDF should be avoided in WHERE ,JOIN,GROUP BY.It perform Like RBAR.It will execute for each row.

You can use Inline TVF and use it in Inner join condition instead of CROSS APPLY. Or if you are using CROSS APPLY then write the TVF logic inside CROSS APPLY itself.

subquery is wrong. It is safe to use TOP 1

(SELECT TOP 1 e.BE_Commercial FROM BE_ENLEVEMENT AS e WHERE e.BE_Numero_BE=x.xIdBe)

You should avoid sub query too. Use LEFT JOIN instead. JOIN is more performant than sub query.

What is the use ROW_NUMBER() OVER (ORDER BY resIdOf) in your query.I do not see it use in INSERT.If it is really so then you can avoid expensive WINDOW Function.

You can take the Screenshot with tool tip or save the Plan of existing query.

Then you can do the change one by one .On each step there will be improvement.