2
\$\begingroup\$

We have recently migrated from SQL Server 2008 R2 to SQL Server 2017 web edition. I am not a SQL developer but enthuses to learn.

Since many of our scrips are of old SQL format, I am searching for some alternative to leverage new T-SQL formats to help optimize the codes.

Below is one example which I believe can be modified using some different approach. It will be helpful for me if someone suggest me some approaches so that I can learn in detail about them.

Below query is for report it uses UNION ALL, the first part fetches data of ID for which payments are done incorrectly marked as 'recoup' and second part fetched data for the same ID it amount of payment required to be done.

SELECT TOP 200 'Recoup',ERH_DOC_DATE,ERH_DOCNO,ERH_RECV_FROM,ERD_BLH_ID,BLD_CODE,A.ERD_ALLOWED_AMT,A.ERD_PAID_INS_AMT
FROM TRN_ERA_HEAD,TRN_ERA_DET A,TRN_BILLING_DET
WHERE ERH_STATUS = 'P' AND ERH_BOOL_INACTIVE = 0 AND A.ERD_BOOL_RECALLED=1 AND
A.ERD_ALLOWED_AMT <> 0 AND A.ERD_PAID_INS_AMT <> 0 AND
ERH_ID = A.ERD_ERH_ID AND
A.ERD_BLD_ID = BLD_ID AND
NOT EXISTS (SELECT B.ERD_ID FROM TRN_ERA_DET B WHERE A.ERD_ERH_ID = B.ERD_ERH_ID AND A.ERD_ID <> B.ERD_ID AND B.ERD_BLD_ID = A.ERD_BLD_ID AND B.ERD_BOOL_RECALLED = 0 AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT)
AND ERH_DOC_DATE>='01-01-2021' AND ERH_DOC_DATE<='05-31-2021' 
UNION ALL 
SELECT TOP 200 'Payment',EH1.ERH_DOC_DATE,EH1.ERH_DOCNO, EH1.ERH_RECV_FROM, ED1.ERD_BLH_ID,BD1.BLD_CODE,ED1.ERD_ALLOWED_AMT, ED1.ERD_PAID_INS_AMT FROM 
( SELECT ERD_ERH_ID ERDERH_ID, ERD_BLH_ID erdblh_id, a.erd_bld_id bld, A.ERD_ALLOWED_AMT, A.ERD_PAID_INS_AMT FROM TRN_ERA_HEAD,TRN_ERA_DET A,TRN_BILLING_DET WHERE ERH_STATUS = 'P' AND ERH_BOOL_INACTIVE = 0 AND A.ERD_BOOL_RECALLED=1 AND A.ERD_ALLOWED_AMT <> 0 AND A.ERD_PAID_INS_AMT <> 0 AND ERH_ID = A.ERD_ERH_ID AND A.ERD_BLD_ID = BLD_ID AND NOT EXISTS 
(SELECT B.ERD_ID FROM TRN_ERA_DET B WHERE A.ERD_ERH_ID = B.ERD_ERH_ID AND A.ERD_ID <> B.ERD_ID AND B.ERD_BLD_ID = A.ERD_BLD_ID AND B.ERD_BOOL_RECALLED = 0 AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT) AND ERH_DOC_DATE>='01-01-2021'
AND ERH_DOC_DATE<='05-31-2021') AS RECOUPS, 
TRN_ERA_HEAD EH1,TRN_ERA_DET ED1, TRN_BILLING_DET BD1 WHERE RECOUPS.ERDERH_ID < ED1.ERD_ERH_ID AND ED1.ERD_BLD_ID = RECOUPS.BLD AND EH1.ERH_ID=ED1.ERD_ERH_ID AND ED1.ERD_BLD_ID=BD1.BLD_ID AND EH1.ERH_TRNTYPE='IN' AND EH1.ERH_STATUS = 'P' AND EH1.ERH_BOOL_INACTIVE = 0 AND ED1.ERD_BOOL_RECALLED=0 AND ED1.ERD_ALLOWED_AMT <> 0 AND ED1.ERD_PAID_INS_AMT <> 0;

Please suggest me some different approaches. Thank you in advance.

\$\endgroup\$
3
  • \$\begingroup\$ use of TOP should have a ORDER BY clause. Reference \$\endgroup\$
    – Squirrel
    May 16 at 13:21
  • \$\begingroup\$ FROM TRN_ERA_HEAD,TRN_ERA_DET A,TRN_BILLING_DET Should use proper join syntax like FROM TRN_ERA_HEAD JOIN TRN_ERA_DET A ON ... \$\endgroup\$
    – Squirrel
    May 16 at 13:25
  • \$\begingroup\$ @Squirrel That's valid feedback - it belongs in an answer :) \$\endgroup\$
    – Reinderien
    May 16 at 14:12
1
\$\begingroup\$

It seems you were using SQL-89 syntax, you should consider using SQL-92 syntax all the way. Using SQL-92 syntax would add more clarity to the query, which is a major advantage. For example, if we take the joins part from your query.

current ANSI SQL-89 syntax :

SELECT TOP 200 
    'Recoup'
,   ERH_DOC_DATE
,   ERH_DOCNO
,   ERH_RECV_FROM
,   ERD_BLH_ID
,   BLD_CODE
,   A.ERD_ALLOWED_AMT
,   A.ERD_PAID_INS_AMT
FROM 
    TRN_ERA_HEAD
,   TRN_ERA_DET A
,   TRN_BILLING_DET
WHERE 
    ERH_STATUS = 'P' 
AND ERH_BOOL_INACTIVE   = 0 
AND A.ERD_BOOL_RECALLED = 1
AND A.ERD_ALLOWED_AMT <> 0 
AND A.ERD_PAID_INS_AMT <> 0
AND ERH_ID = A.ERD_ERH_ID 
AND A.ERD_BLD_ID = BLD_ID 
AND NOT EXISTS (
    SELECT B.ERD_ID 
    FROM 
        TRN_ERA_DET B 
    WHERE 
            A.ERD_ERH_ID = B.ERD_ERH_ID 
        AND A.ERD_ID <> B.ERD_ID 
        AND B.ERD_BLD_ID = A.ERD_BLD_ID 
        AND B.ERD_BOOL_RECALLED = 0 
        AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT 
        AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
    )
AND ERH_DOC_DATE >='01-01-2021' 
AND ERH_DOC_DATE<='05-31-2021' 

using ANSI SQL-92 syntax :

SELECT TOP 200 
    'Recoup'
,   TEH.ERH_DOC_DATE
,   TEH.ERH_DOCNO
,   TEH.ERH_RECV_FROM
,   TED.ERD_BLH_ID
,   TBD.BLD_CODE
,   TED.ERD_ALLOWED_AMT
,   TED.ERD_PAID_INS_AMT
FROM 
    TRN_ERA_HEAD TEH
INNER JOIN TRN_ERA_DET TED ON TEH.ERH_ID = TED.ERD_ERH_ID
    AND TEH.BLD_ID = TED.ERD_BLD_ID
    AND TED.ERD_BOOL_RECALLED = 1
    AND TED.ERD_ALLOWED_AMT <> 0 
    AND TED.ERD_PAID_INS_AMT <> 0
INNER JOIN TRN_BILLING_DET TBD ON TED.ERD_BLD_ID = TBD.BLD_ID
WHERE 
    TEH.ERH_DOC_DATE >='01-01-2021' 
AND TEH.ERH_DOC_DATE<='05-31-2021' 
AND TEH.ERH_STATUS = 'P' 
AND TEH.ERH_BOOL_INACTIVE   = 0 
AND NOT EXISTS (
    SELECT B.ERD_ID 
    FROM 
        TRN_ERA_DET B 
    WHERE 
            TED.ERD_ERH_ID = B.ERD_ERH_ID 
        AND TED.ERD_ID <> B.ERD_ID 
        AND B.ERD_BLD_ID = TED.ERD_BLD_ID 
        AND B.ERD_BOOL_RECALLED = 0 
        AND TED.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT 
        AND TED.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
)
ORDER BY TEH.ERH_DOC_DATE ASC;

if you compare both versions, you would see the SQL-92 would be more readable, and easier to understand.

Aliasing

When you include multiple tables in the query, you should always use a proper alias to each table (including the source table). This would add more clarity to the query. As for your current query lakes that, because of that, we don't know much of the presented columns, and we can't guess the table's columns. even if the column itself has a prefix naming of the parent table, that's not enough, each column needs to have an alias to the parent table, otherwise, it will add more time and efforts figuring things out.

The other part of aliasing, is the alias naming convention. In simple queries, it would mostly be fine if you use some letters, as it's easy to navigate. However, with complex queries, it's recommended to pick a better naming for each table. It's common to have the table name initials, or the first three or four letters from its name. Some developers stick with the full table name, which is also serves the same goal. At the end, your goal is to pick a clear and readable alias that would represent the table so you won't have to go back and check the table name just to get a better view of the query.

Query Refactoring

The current query works on three tables TRN_ERA_HEAD, TRN_ERA_DET, and TRN_BILLING_DET, and I can see the difference between the two queries is the ERH_TRNTYPE and ERD_BOOL_RECALLED which what distinguish between Recoup and Payment. If that's is the case, then we could avoid the UNION ALL and do it in one query, something like this :

SELECT TOP 200
CASE WHEN TEH.ERH_TRNTYPE = 'IN' AND TEH.ERD_BOOL_RECALLED = 0 THEN 'Payment' ELSE 'Recoup' END PaymentReportType
,   TEH.ERH_DOC_DATE
,   TEH.ERH_DOCNO
,   TEH.ERH_RECV_FROM
,   TED.ERD_BLH_ID
,   TBD.BLD_CODE
,   TED.ERD_ALLOWED_AMT
,   TED.ERD_PAID_INS_AMT
FROM 
    TRN_ERA_HEAD TEH
INNER JOIN TRN_ERA_DET TED  ON TED.ERD_ERH_ID  = TEH.ERH_ID
INNER JOIN TRN_BILLING_DET TBD ON TED.ERD_BLD_ID = TBD.BLD_ID
WHERE 
    TEH.ERH_DOC_DATE >='01-01-2021' 
AND TEH.ERH_DOC_DATE <='05-31-2021' 
AND TEH.ERH_STATUS = 'P' 
AND TEH.ERH_BOOL_INACTIVE   = 0
AND TED.ERD_ALLOWED_AMT <> 0 
AND TED.ERD_PAID_INS_AMT <> 0
AND NOT EXISTS (
    SELECT B.ERD_ID 
    FROM 
        TRN_ERA_DET B 
    WHERE 
            TED.ERD_ERH_ID = B.ERD_ERH_ID 
        AND TED.ERD_ID <> B.ERD_ID 
        AND B.ERD_BLD_ID = TED.ERD_BLD_ID 
        AND B.ERD_BOOL_RECALLED = 0 
        AND TED.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT 
        AND TED.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
)
ORDER BY TEH.ERH_DOC_DATE ASC;

The example is just to help you reducing the query complexity. So, you will need to to work on it against the data until you get the expected results.

\$\endgroup\$
1
\$\begingroup\$

To start, here is a potential formatting that makes the query less of a tragedy to read:

SELECT TOP 200
    'Recoup', ERH_DOC_DATE, ERH_DOCNO, ERH_RECV_FROM, ERD_BLH_ID,
    BLD_CODE, A.ERD_ALLOWED_AMT, A.ERD_PAID_INS_AMT
FROM TRN_ERA_HEAD, TRN_ERA_DET A, TRN_BILLING_DET
WHERE ERH_STATUS = 'P' 
    AND ERH_BOOL_INACTIVE = 0 
    AND A.ERD_BOOL_RECALLED = 1 
    AND A.ERD_ALLOWED_AMT <> 0 
    AND A.ERD_PAID_INS_AMT <> 0 
    AND ERH_ID = A.ERD_ERH_ID 
    AND A.ERD_BLD_ID = BLD_ID 
    AND NOT EXISTS (
        SELECT B.ERD_ID 
        FROM TRN_ERA_DET B
        WHERE A.ERD_ERH_ID = B.ERD_ERH_ID 
            AND A.ERD_ID <> B.ERD_ID
            AND B.ERD_BLD_ID = A.ERD_BLD_ID
            AND B.ERD_BOOL_RECALLED = 0
            AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT
            AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
    )
    AND ERH_DOC_DATE >= '01-01-2021'
    AND ERH_DOC_DATE <= '05-31-2021' 
UNION ALL 
SELECT TOP 200 
    'Payment', EH1.ERH_DOC_DATE, EH1.ERH_DOCNO, EH1.ERH_RECV_FROM,
    ED1.ERD_BLH_ID, BD1.BLD_CODE, ED1.ERD_ALLOWED_AMT, ED1.ERD_PAID_INS_AMT
FROM (
    SELECT
        ERD_ERH_ID ERDERH_ID, ERD_BLH_ID erdblh_id, a.erd_bld_id bld,
        A.ERD_ALLOWED_AMT, A.ERD_PAID_INS_AMT
    FROM TRN_ERA_HEAD, TRN_ERA_DET A,TRN_BILLING_DET
    WHERE ERH_STATUS = 'P'
        AND ERH_BOOL_INACTIVE = 0
        AND A.ERD_BOOL_RECALLED = 1
        AND A.ERD_ALLOWED_AMT <> 0 
        AND A.ERD_PAID_INS_AMT <> 0
        AND ERH_ID = A.ERD_ERH_ID
        AND A.ERD_BLD_ID = BLD_ID
        AND NOT EXISTS (
            SELECT B.ERD_ID
            FROM TRN_ERA_DET B
            WHERE A.ERD_ERH_ID = B.ERD_ERH_ID
                AND A.ERD_ID <> B.ERD_ID
                AND B.ERD_BLD_ID = A.ERD_BLD_ID
                AND B.ERD_BOOL_RECALLED = 0
                AND A.ERD_ALLOWED_AMT = B.ERD_ALLOWED_AMT
                AND A.ERD_PAID_INS_AMT = B.ERD_PAID_INS_AMT
        )
        AND ERH_DOC_DATE >= '01-01-2021'
        AND ERH_DOC_DATE <= '05-31-2021'
    ) AS RECOUPS, 
    TRN_ERA_HEAD EH1,
    TRN_ERA_DET ED1, 
    TRN_BILLING_DET BD1 
WHERE RECOUPS.ERDERH_ID < ED1.ERD_ERH_ID
    AND ED1.ERD_BLD_ID = RECOUPS.BLD
    AND EH1.ERH_ID = ED1.ERD_ERH_ID
    AND ED1.ERD_BLD_ID = BD1.BLD_ID
    AND EH1.ERH_TRNTYPE = 'IN'
    AND EH1.ERH_STATUS = 'P'
    AND EH1.ERH_BOOL_INACTIVE = 0
    AND ED1.ERD_BOOL_RECALLED = 0
    AND ED1.ERD_ALLOWED_AMT <> 0
    AND ED1.ERD_PAID_INS_AMT <> 0;

Aside from the valid points that @Squirrel has already raised:

\$\endgroup\$

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.