0

how can i optimize this code in SQL

INSERT INTO #ActivePlayers ([PlayerId])
        SELECT DISTINCT([OwnerSID]) [PlayerId]            
        FROM [WarehouseMgmt].[FactLoginTrans] FLT
        JOIN [WarehouseMgmt].[DimPlayer] DP ON DP.[Id] = FLT.[OwnerSID]
        WHERE [IsSystemUser]=0
        AND [OwnerSID]>0
        AND [WarehouseReports].[ConvertfromUTCtoTZ] (FLT.[LogonTime],@ZoneCode) BETWEEN DATEADD(month, -13, @Date) AND DATEADD(month, -1, @Date)

What's the best option for [WarehouseReports].[ConvertfromUTCtoTZ] (FLT.[LogonTime],@ZoneCode) BETWEEN DATEADD(month, -13, @Date) AND DATEADD(month, -1, @Date) , because this take me some time and i repeat this line in few SQL queries. How to put this into temp table and use it later in query ?

Improve this question
2
  • Since @ZoneCode is a constant instead of passing FLT.LogonTime to the function for every row, you could just pass the constant @Date, thus only calling the function once? Alternatively, if you post the definition it is possible that you could convert the function to an inline table valued function, which performs significantly better than a scalar function. Commented Aug 26, 2015 at 8:36
  • Can you replace the ConvertFromUTCtoTZ on the table columns to converting the DateAdd() values from TZ to UTC ? Using FLT.logonTime between ConvertUTCToTZ(date1) and ConvertUtcToTZ(Date2) will utilize indexes on Flt.LogonTime. Commented Aug 26, 2015 at 10:24

2 Answers 2

Reset to default
2

To optimize a SQL query you need to understand where the bottlenecks occur. Read How to analyse SQL Server performance.

The most important factor for SQL optimization is having a SARG-able predicate. In other words your WHERE clause and the JOINs must be able to use indexes. As is, your query is unsargable. An index on [WarehouseMgmt].[DimPlayer]([Id]) is required, but I'm pretty sure you already have one. Now look at your WHERE clause:

WHERE [IsSystemUser]=0
AND [OwnerSID]>0
AND [WarehouseReports].[ConvertfromUTCtoTZ] (FLT.[LogonTime],@ZoneCode) BETWEEN 
    DATEADD(month, -13, @Date) AND DATEADD(month, -1, @Date)

This cannot be made to use an index. The first two conditions can be discarded because of low carnality (too many rows will qualify). Which leaves the range predicate on the date range. By asking to convert the stored timezone into the parameter time zone you throw away any possible chance at optimizing the query because of the UDF. You should do the opposite:

  • make sure the [LogonTime] is store UTC (always store any date/time in the database as UTC)
  • convert the @date from user timezone to UTC:
    @utcdate = ConvertfromTZtoUTC] (@date,@ZoneCode)
  • express the range on [LogonTime]:
    LogonTime BETWEEN DATEADD(month, -13, @utcdate) AND DATEADD(month, -1, @utcdate)
  • have a covering index ([LogonTime]) INCLUDE ([IsSystemUser], [OwnerSID])

Start reading Designing Indexes.

In future when asking database queries, always include a complete schema of your tables, including all indexes.

Improve this answer
Sign up to request clarification or add additional context in comments.

1 Comment

What you mean by ''convert the @date from user timezone to UTC and express the range on [LogonTime] ''
-1

May be you have to do something like (warning, I write metacode, be sure to check your SQL server docs!)

 ALTER TABLE [WarehouseMgmt].[FactLoginTrans]
   ADD COLUMN BOOLEAN IsInPeriodOfInterest

UPDATE [WarehouseMgmt].[FactLoginTrans]
   SET IsInPeriodOfInterest = [WarehouseReports].[ConvertfromUTCtoTZ](FLT.[LogonTime], @ZoneCode)
       BETWEEN DATEADD(month, -13, @Date) AND DATEADD(month, -1, @Date)

And alter your further requests to refer to this field?

Or you can, of course, 

CREATE TEMPORARY TABLE (YOUR_PRIMARY_KEY, boolean IsInPeriodOfInterest)
SELECT YOUR_PRIMARY_KEY, [WarehouseReports].[ConvertfromUTCtoTZ](FLT.[LogonTime],@ZoneCode)
       BETWEEN DATEADD(month, -13, @Date) AND DATEADD(month, -1, @Date)
  FROM [WarehouseMgmt].[FactLoginTrans] FLT

and than join with it.

Improve this answer

2 Comments

This is neither any more efficient, or good practice (also not valid syntax but you have put in a caveat for this). Updating a table to set a field you want in an ad hoc query is not even remotely thread safe. What if someone else runs a similar query with a different zone half way through when you are running it? The results will be nonsense.
You right, adding a column is a bad practice, sory. But the case with temporary table will work, if database supports them on per-connectio basis. If some other process will modify the data while we are making manipulations with it, we are going to ge unpredicted behaviour anyway.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.