asked on

What is best for SQL Error trapping. Using DYNAMICS SQL or single line execution.

I have code that imports sales orders into an ERP solution. There is an order header and then order detail lines. The lines are imported first. The problem I have discovered is that even though I have code that writes to a log file indicating success or failure I have orders that simply do not get imported and there is nothing in the log file to indicate failure. So when I check the log I will see order 1000,1001, 1003,1005, etc. So 1002 simply is just not there. My question here and I am seeking your expert opinions is, should I not use DYNAMICS SQL that combines all the inserts into one execute statement and insert one line at a time and capture the log for each insertion? I included the code to give you a sense of what is being done.

      -- Build the dynamics sql statment to insert the lines
      SELECT @dynamicSQL += 'EXECUTE taSopLineIvcInsert @I_vSOPTYPE=' + LTRIM(RTRIM(CONVERT(CHAR(10),H.DOCTYPE_SOPH))) + ',@I_vSOPNUMBE=''' + LTRIM(RTRIM(H.SOPNUMBE_SOPH)) + ''',@I_vCUSTNMBR=''' +
            LTRIM(RTRIM(CUSTNUMB_SOPH)) + ''',@I_vDOCDATE=''' + CONVERT(CHAR(10),DOCDATE_SOPH,111) + ''',@I_vLOCNCODE=''' + LTRIM(RTRIM(LOCNCODE_SOPH)) + ''',@I_vITEMNMBR=''' + LTRIM(RTRIM(ITEMNUMB_SOPL)) +
            ''',@I_vUNITPRCE=' + LTRIM(RTRIM(CONVERT(CHAR(10),CAST(UNITPRCE_SOPL AS NUMERIC(10,2))))) + ',@I_vQUANTITY=' + '0' + ',@I_vPRCLEVEL=''' + LTRIM(RTRIM(@I_PRCLEVEL)) +
            ''',@I_vQTYTBAOR=' + LTRIM(RTRIM(CONVERT(CHAR(10),CAST(CUST_QTY_SOPL AS NUMERIC(10,2))))) + ',@I_vPRSTADCD=''' + LTRIM(RTRIM(SHIP_TO_CODE_SOPH)) +
            ''',@I_vReqShipDate=''' + CONVERT(CHAR(10),CSTSHDAT_SOPL,111) +
            ''',@I_vSHIPMTHD=''' + LTRIM(RTRIM(SHIPMTHD_SOPH)) + ''',@I_vUpdateIfExists=' + LTRIM(RTRIM(CONVERT(CHAR(10),@I_UpdateIfExists))) + ',@I_vDEFPRICING=' + LTRIM(RTRIM(CONVERT(CHAR(10),@I_DEFPRICING))) +
            ',@I_vDEFEXTPRICE=' + LTRIM(RTRIM(CONVERT(CHAR(10),@I_DEFEXTPRICE))) + ',@I_vCURNCYID=''' + LTRIM(RTRIM(@I_CURNCYID)) + ''',@I_vUOFM=''' + LTRIM(RTRIM(UOFM_SOPL)) +
            ''',@O_iErrorState=@O_ErrorState OUTPUT,@oErrString=@O_ErrorString OUTPUT '
            FROM [POWMATQP].[dbo].[SOPLINES] L
            INNER JOIN [POWMATQP].[dbo].[SOPHEADR] H ON L.SOPNUMBE_SOPL=H.SOPNUMBE_SOPH AND L.DOCTYPE_SOPL=H.DOCTYPE_SOPH
            WHERE SOPNUMBE_SOPL=@I_SOPNUMBE AND DOCTYPE_SOPL=@I_SOPTYPE AND MARK_TO_ORDER_SOPL=1 and SOP_XFER_TO_GP_SOPL=0

      -- insert the lines
      execute sp_executesql @dynamicSQL

Arana (G.P.)

to start I would write a log line before calling the sp and after, just to of them maybe containing the order number, if you get this logs then you can go deeper and see what it is in your sql that is causing the problem, if not then the problem is somewhere before calling the SP

ste5an

What SQL dialect? Oracle and T-SQL have excpetion handling constucts..

slightwv (䄆 Netminder)

I never understood the desire to jump to dynamic SQL when straight SQL will work more times than it won't.

Personally I would go into a loop and make individual calls to taSopLineIvcInsert. That gives you options to perform extra steps that might be necessary. Possibly data checks before calling the procedure and the extra logging you might need to help troubleshoot the issue of missing orders?

I also don't see a need for calls to both RTRIM and LTRIM when there is a perfectly good single TRIM function.

So LTRIM(RTRIM(H.SOPNUMBE_SOPH)) can become TRIM(H.SOPNUMBE_SOPH)

rwheeler23

ASKER

taSopLineIvcInsert and others like it limit what I can do. They are not my creation but are mandated by the ERP solution. The ,@O_iErrorState=@O_ErrorState OUTPUT,@oErrString=@O_ErrorString OUTPUT parameters are what I will get back from each call. I would agree that it would be better to create a loop for each line on the order and log what is returned. The LTRIM and RTRIM is muscle memory. I will change that to TRIM.

ste5an

Do you want each value of the O_ErrorState and O_ErrorString? Then you need to run this in a cursor or built an much uglier batch, where you add pushing those values to a temporary table.
Furthermore, those output values indicate that you'll won't get an exception, when there went something wrong in the procedure.

rwheeler23

ASKER

Yes, I will restructure this using a cursor so I can create the loop. The errors I am hoping to trap are due to constraints of the ERP solution. For example, an order comes along but the customer is over their credit limit. The reason for this is because every so often an order simply does not import. My suspicion is that some ERP rule is being violated. I need to be able to alert the user as to what is the problem. That is contained in those error codes.

rwheeler23

ASKER

I prefer not to use cursors whenever possible. That is why I had used dynamic SQL. I am currently restructuring this to loop through a cursor and execute the stored procedure for each line. I will let you know how this works out.

slightwv (䄆 Netminder)

What is your aversion to cursors? Everything in SQL uses cursors either explicit or implicit. The select you executed to get the results is a cursor.

rwheeler23

ASKER

Cursors offer poor performance and the potential of creating locks is high.

slightwv (䄆 Netminder)

Depends on what you are doing. In this case, you are performing individual actions row by row and want to capture the results on each action. Kind of screams for a cursor.

I think this says it better than I could:

Poor use of cursors falls into two broad categories. The first is badly written cursors. Sadly, bad programming can be found in virtually any environment and only proper training can solve this problem. The second category of poor cursors consists of developers who use cursors when a better solution is available.

https://blog.learningtree.com/are-cursors-evil-part-one-how-to-declare-cursors-in-transact-sql/

rwheeler23

ASKER

Point well taken. In this case, record locking would not be an issue because all the records are new. Also, most orders consist of only a handful of records so performance should not be an issue. My task for tomorrow is to convert this task to use a cursor.

This question needs an answer!

Become an EE member today

7 DAY FREE TRIAL

Members can start a 7-Day Free trial then enjoy unlimited access to the platform.

View membership options

Learn why we charge membership fees

We get it - no one likes a content blocker. Take one extra minute and find out why we block content.