1
\$\begingroup\$

I have a list of about 55k Accounts that I am cleaning up. 95% of them are from US/Canada which is what I'm most concerned with. I am going through record by record and will perform various checks and fill in or correct City, State, Zip, Country. I have a table with every City, State, and Zip for US and Canada that I check against. For this part, maybe 20% will be processed after grabbing the info.

I am doing a kind of proof of concept right now, and it's taking forever. I just hit the 1hr mark with 24,500 (Less than 7 records/sec) . I didn't expect this to take so long. Next will be running a similar process on the Contacts, and there are over 400k contacts. I'm afraid how long that will take!!

Is there a better way to do this to decrease the time taken?

Here is the code right now:

-- MS SQL Server 15.0.2070.41
ALTER PROCEDURE [dbo].[SetLocationInfo]
AS
BEGIN

DECLARE
    @maxRecords int = 0,
    @cnt        int = 1,
    @city       varchar(255),
    @state      varchar(255),
    @zip        varchar(255),
    @country    varchar(255)


SELECT TOP(1) @maxRecords = id2 FROM Account ORDER BY Id2 DESC

-- BEGIN LOOP
WHILE @cnt <= @maxRecords
BEGIN
    -- Resets Variables
    SET @city = NULL
    SET @state = NULL
    SET @zip = NULL
    SET @country = NULL

    -- Pulls BILLING Address Info from Record
    SELECT
        @city = [BillingCity],
        @state = [BillingState],
        @zip = [BillingPostalCode],
        @country = [BillingCountry]
      FROM [Account] WHERE Id2 = @cnt

    -- Searches for and Sets BILLING Country
    IF (@country IS NULL) OR ((@country != 'United States') AND (@country != 'Canada'))
    BEGIN
        SELECT TOP(1) @country = Country FROM [CityStateInfo] 
          WHERE 
          (City = @city AND State_abbr = @state) OR 
          (City = @city AND Zip = LEFT(@zip,5)) OR
          (Zip = LEFT(@zip,5))

        IF @country IS NOT NULL
            UPDATE Account SET [BillingCountry] = @country WHERE Id2 = @cnt
    END


    -- Resets Variables
    SET @city = NULL
    SET @state = NULL
    SET @zip = NULL
    SET @country = NULL

    -- Pulls SHIPPING Address Info from Record
    SELECT
        @city = [ShippingCity],
        @state = [ShippingState],
        @zip = [ShippingPostalCode],
        @country = [ShippingCountry]
      FROM [Account] WHERE Id2 = @cnt

    -- Searches for and Sets SHIPPING Country
    IF (@country IS NULL) OR ((@country != 'United States') AND (@country != 'Canada'))
    BEGIN
        SELECT TOP(1) @country = Country FROM [CityStateInfo] 
          WHERE 
          (City = @city AND State_abbr = @state) OR 
          (City = @city AND Zip = LEFT(@zip,5)) OR
          (Zip = LEFT(@zip,5))

        IF @country IS NOT NULL
        BEGIN
            PRINT CAST(@cnt AS varchar(10)) + ': ' + @country
            UPDATE Account SET [ShippingCountry] = @country WHERE Id2 = @cnt
        END
    END


    -- Increments Counter
    SET @cnt = @cnt + 1
END
-- END LOOP

END
\$\endgroup\$
1
  • \$\begingroup\$ What specific SQL dialect are you using? May be such thing could be done with a single WITH statement. \$\endgroup\$ Mar 31 '20 at 6:24
2
\$\begingroup\$

You should be able to do this with a simple update command, no loops at all, something like this:

UPDATE Account SET [BillingCountry]  = << some expression >>
where isnull(BillingCountry,'') not in ( 'United States', 'Canada' )

I would recommend writing a function to compute the country, it probably makes it easier to test the results are correct before running the update. The function would take as parameters the City, State and Zipcode ( and perhaps the current value of Country if you you want that to be the result if it cannot be computed ), and return the computed country ( based on the CityStateInfo table you have ).

This should run pretty fast, within a few seconds, provided the CityStateInfo table is appropriately indexed ( on both City,State_abbr and also on Zip ).

Your expression for matching seems to be redundant:

   (City = @city AND State_abbr = @state) OR 
          (City = @city AND Zip = LEFT(@zip,5)) OR
          (Zip = LEFT(@zip,5))

The middle line can be removed.

\$\endgroup\$
4
  • \$\begingroup\$ I actually did this for my Contact table. I added Indicies to the CityStateInfo table. Copied what I need into a temp table, about 48k rows. Ran this update (31,500 rows) and it took 42min. UPDATE #Contact_Address SET [MailingCountry] = (SELECT TOP(1) Country FROM [ULTRA].[dbo].[CityStateInfo] WHERE City = [MailingCity] AND (State_long = [MailingState] OR State_abbr = [MailingState]) AND Zip = LEFT([MailingPostalCode],5)) WHERE [MailingCountry] IS NULL AND [MailingCity] IS NOT NULL AND [MailingState] IS NOT NULL AND [MailingPostalCode] IS NOT NULL \$\endgroup\$
    – Dizzy49
    Apr 2 '20 at 20:51
  • \$\begingroup\$ @Dizzy49 I would check the execution plan to see if the indices are being used. It may be best to check the Zipcode first ( using that index ), then if that fails (to yield a country) check the City/State. I don't think it should be taking 42 minutes. \$\endgroup\$ Apr 2 '20 at 21:17
  • \$\begingroup\$ Not sure what was going on then, but every other time I've run it it's taken 12-14min which is more reasonable. I'm processing 3-4x as much as my last table, in a total of less than 30min vs HOURS :D I think I found where to view the Execution Plan, but I have no idea how to read it :( \$\endgroup\$
    – Dizzy49
    Apr 3 '20 at 16:39
  • \$\begingroup\$ @Dizzy49 If the plan says it's scanning a table, that's bad. Another approach might be to construct a view (or query) with the correct values ( using two left joins to the CityStateInfo table ), then update the Account table by selecting the correct value from the view (or query) with an id field. \$\endgroup\$ Apr 3 '20 at 20:04
3
\$\begingroup\$

Please update your questions with the relevant tags, although it is obvious you are using SQL Server here.

If you run an execution plan in your query window, you will be able to see which specific bits are taking a lot time and whether SQL Server is using any indexes etc.

It would be good if you gave a data sample and the table structure. Here are some ideas though.

Is there a reason why you are using a WHILE loop instead of a cursor ? The idea here is that you start with ID 1 and increment until you reach the max ID but there must probably be gaps in your table... the approach is possibly wasteful if you are trying to update rows that don't even exist.

Rather than work on the table Account directly (which may be in use or subject to locks), I would rather work on a partial copy using a temporary table.

Step 1: create a temporary table with all the records from Account you want to fix, matching some criteria eg country empty

SELECT *
INTO #Account
FROM Account
WHERE <add your criteria here>

Step 2: perform all your operations on that temporary table #Account

Step 3: update the original table based on the temporary table (UPDATE FROM):

UPDATE Account
SET
    Account.ShippingCountry = #Account.ShippingCountry,
    Account.BillingCountry = #Account.BillingCountry,
    ...
FROM #Account
WHERE
  #Account.id2 = Account.id2

Here I am assuming that id2 is indeed an identity column that uniquely identifies each record. If it isn't, but instead is some kind of unindexed integer value, then each UPDATE requires a full table scan and there is a performance hit here. Again, can't tell without knowing the table structure for sure.

If all this takes too much time, you can process the jobs in small chunks, for example pull 1000 records at a time using SELECT TOP 1000, and repeat until the table is cleaned.

In short, updating a large table can be very slow if you cannot rely on some index, using an appropriate WHERE clause.

Important remark regarding the temporary table approach: doing SELECT INTO creates a copy of the data but does not include the complete table structure, that is indexes, constraints. That mean the id column will be an ordinary integer and not a unique, indexed field. So I would add the index manually on that field to speed the UPDATE FROM.

\$\endgroup\$
2
  • \$\begingroup\$ "Please update your questions with the relevant tags, although it is obvious you are using SQL Server here." should be a comment on the question and not be part of your review. \$\endgroup\$
    – BCdotWEB
    Mar 31 '20 at 15:09
  • \$\begingroup\$ Thank you for your reply! Let's see. I've never used a Cursor before. I've read mixed things on them, so I stuck with what I know (I'm coming from traditional programming). The table had an id column already which was like a uuid, so I added id2 to use for the loop and make it easier to do on-off updates. I'm also not great with indicies (cringe). I learned a lot from your reply, thank you! \$\endgroup\$
    – Dizzy49
    Mar 31 '20 at 22:42

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.