0
\$\begingroup\$

The below query is used to return a dataset for a SSRS report. It works fine and is quick enough but I am looking for any improvements that could be made.

The client has asked to see all transfers between sites and the report should look like:

Site1

  • Site 2
  • Site 3

Site2

  • Site 1
  • Site 3

Each row should also show the value of the transfer to that site and from that site. Duplicated data is expected I.E If Site 1 transferred goods to Site 2 then Site 2 should display the same value in the from column. If no transfer takes place the site should be visible but with a value of 0 in both To and from columns.

The Exists clause must remain as well as it is used to workout descendants of selected site.

WITH Transfers_CTE(FromSiteNo
                  ,ToSiteNo
                  ,Value)
     AS (
     SELECT Transfers.FromSiteNo
           ,Transfers.ToSiteNo
           ,SUM(Transfers.[Value]) AS Value
     FROM dbo.Transfers
     WHERE Transfers.MoveDate BETWEEN @SessionDateFrom AND @SessionDateTo
     GROUP BY Transfers.FromSiteNo
             ,Transfers.ToSiteNo)
     SELECT CS.No AS SiteNo
           ,CS.Name AS SiteName
           ,JoinedSites.No AS OtherSiteNo
           ,JoinedSites.Name AS OtherSiteName
           ,ISNULL(ToTable.[Value],0) AS ToValue
           ,ISNULL(FromTable.[Value],0) AS FromValue
     FROM dbo.CfgSites AS CS
          FULL OUTER JOIN dbo.CfgSites AS JoinedSites ON CS.No != JoinedSites.No
          LEFT JOIN Transfers_CTE AS ToTable ON ToTable.FromSiteNo = CS.No
                                                AND ToTable.ToSiteNo = JoinedSites.No
          LEFT JOIN Transfers_CTE AS FromTable ON FromTable.ToSiteNo = CS.No
                                                  AND FromTable.FromSiteNo = JoinedSites.No
     WHERE EXISTS
(
    SELECT DescendantSites.Descendant
    FROM dbo.DescendantSites
    WHERE DescendantSites.Parent IN(@SiteNo)
    AND DescendantSites.Descendant = CS.No
)
     ORDER BY CS.No;

Subset DDL for CfgSites and Transfer View that only includes required fields

/****** Object:  Table [dbo].[CfgSites]    Script Date: 13/04/2018 08:47:32 ******/

CREATE TABLE [dbo].[CfgSites](
    [No] [smallint] NOT NULL,
    [Name] [varchar](50) NULL)
GO
/****** Object:  View [dbo].[Transfers]    Script Date: 13/04/2018 08:47:32 ******/

CREATE VIEW [dbo].[Transfers]
AS
SELECT [SiteNo] AS FromSiteNo
    ,[Site2No] AS ToSiteNo
    ,[Date] AS MoveDate
    ,[Value]
FROM PluMovement
WHERE MoveType = 4
    AND Processed = 1
\$\endgroup\$
  • \$\begingroup\$ Can you elaborate on this part: WHERE DescendantSites.Parent IN(@SiteNo) I'm assuming this is a delimited list? If not, you can simply use = \$\endgroup\$ – scsimon Apr 12 '18 at 13:23
  • \$\begingroup\$ @scsimon the parameter SiteNo can be multiple values or just a single entry. It allows the customer to view any selected sites they want. \$\endgroup\$ – NinjaArekku Apr 12 '18 at 13:52
0
\$\begingroup\$

Here is an attempt at a rewrite, but you didn't provide any DDL or DML so there is no way for me to test it. I'll list reasons for the changes below.

WITH Transfers_CTE (FromSiteNo, ToSiteNo, Value) AS
  (
    SELECT      Transfers.FromSiteNo,
                Transfers.ToSiteNo,
                SUM( Transfers.Value ) AS Value
      FROM      dbo.Transfers
      WHERE     Transfers.MoveDate BETWEEN @SessionDateFrom AND @SessionDateTo
                AND FromSiteNo <> ToSiteNo
      GROUP BY  Transfers.FromSiteNo,
                Transfers.ToSiteNo
  )
  SELECT        FromCS.No AS SiteNo,
                FromCS.Name AS SiteName,
                ToCS.No AS OtherSiteNo,
                ToCS.Name AS OtherSiteName,
                ISNULL( Transfers.Value, 0 ) AS ToFromValue
    FROM        dbo.CfgSites AS FromCS
    LEFT JOIN   Transfers_CTE AS Transfers
      ON FromCS.No = Transfers.FromSiteNo
    LEFT JOIN   dbo.CfgSites AS ToCS
      ON Transfers.ToSiteNo = ToCS.No
    WHERE       EXISTS
        ( SELECT    DescendantSites.Descendant
            FROM    dbo.DescendantSites
            WHERE   DescendantSites.Parent IN ( @SiteNo )
                    AND DescendantSites.Descendant = FromCS.No)
    ORDER BY    FromCS.No
;
  1. Since Cs.No and JoinedSites.No should not be equal, and they are joined to the From & To values of the CTE, I added the filter in the CTE to exclude any transfers where the To and From site matched.
  2. Your Full Outer Join was effectively a left outer join because any results from the Right side table would have been excluded by the Left side reference of your EXISTS clause.
  3. You attempt to join transfers to sites from both directions. However, if A = B, then B = A. So if we get all of the FromSite joins established to CS on the first pass, there is no reason to do a second pass by joining JoinedSites to the FromSite as they will already have found a match. This change also eliminates the near Cartesian product of joining CfgSites to itself.
  4. If we consider the CS table our anchor, understanding it has all of the From Sites included, our problem then becomes how to get the To sites. The relationship between From and To is contained in the Transfers_CTE. Thus we just need to join back to the CfgSites using the ToSiteNo field to find the site information.
  5. The Sum aggregate was at the grain of the Transfer relationship, To & From. This is maintained in the changes, but now there is only one value column, returning the Value that was transferred between From and To.

There is no need to join CfgSites to itself directly, unless you may also need to show any ToFrom combinations that did not get any transfers.

Hopefully this will return your expected results. If not, provide some DDL/DML and I may take another look at it.

\$\endgroup\$
  • \$\begingroup\$ Thanks for the feedback. I realised that I haven't been clear enough in the purpose of the script so i have updated my question with, hopefully, a better reflection on what it has to achieve and included the DDL. In relation to point 1, is it worth adding a filter to an event that will only occur 0.01% of the time (through fault)? Would it impact performance if filtering for something it cant find? \$\endgroup\$ – NinjaArekku Apr 13 '18 at 8:06

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.