HiveBrain v1.2.0
Get Started
← Back to all entries
patternsqlMinor

Listing payments where all payments lines and transactions are closed

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
allarewhereclosedlistingandtransactionspaymentslines

Problem

The goal of this query is to get a list of payments where all the corresponding payment lines and transactions have the status CLOSED. These are one to many relationships. The database is Informix.

I wonder if there is a more efficient way to write this? In its current state it has a lot of repetition which, maybe because my Java background, seems quite ugly to me. Eventually this query will need to be generated through Hibernate, so I'm mostly concerned with its performance.

SELECT p.* FROM payment p
JOIN transaction t
ON t.payment_bk = p.bk
JOIN payment_line pl
on  p.bk = pl.payment_bk
WHERE p.valid AND p.valid_to is null
AND p.date>{ts '2015-05-21 00:00:00'} 
AND p.date 'CLOSED'
AND t.valid and t.valid_to is null)
AND pl.valid and pl.valid_to is null
AND pl.payment_line_status = 'CLOSED'
AND NOT EXISTS (SELECT 1 FROM payment_line WHERE pl.payment_bk = p.bk 
AND pl.payment_line_status <> 'CLOSED'
AND pl.valid and pl.valid_to is null)


valid is a boolean and the dates/timestamps will sometimes be just a day apart, sometimes a couple of months.

Solution

White space

Since you come from a Java background, you will understand the importance of using white space to make the code easier to read. Also it is best to stay consistent with capitalization of keywords. I would go for something like this:

SELECT p.* 
FROM payment p
  JOIN transaction t
    ON t.payment_bk = p.bk
  JOIN payment_line pl
    ON  p.bk = pl.payment_bk
WHERE p.valid IS NULL
  AND p.valid_to IS NULL
  AND p.date>{ts '2015-05-21 00:00:00'} 
  AND p.date 'CLOSED'
      AND t.valid and t.valid_to IS NULL
  )
  AND pl.valid IS NULL
  AND pl.valid_to IS NULL
  AND pl.payment_line_status = 'CLOSED'
  AND NOT EXISTS (
    SELECT 1 
    FROM payment_line 
    WHERE pl.payment_bk = p.bk 
      AND pl.payment_line_status <> 'CLOSED'
      AND pl.valid IS NULL
      AND pl.valid_to IS NULL
  );


So, that already reads a lot better!

Table aliases are handy, but in your case I think your use of single-letter aliases makes the query harder to read. For instance, instead of payment p JOIN transaction t I would suggest something like payment AS pay INNER JOIN transaction AS tran. And payment_line pl could be payment_line AS line or something like that.

Normally I would recommend using Common Table Expressions instead of sub-queries, but since this will eventually be done by Hibernate it would not matter too much right now.

For clarity's sake, I would avoid to chain WHERE conditions like this:

WHERE p.valid AND p.valid_to IS NULL


I missed those at first, so that gives you an idea why.

Code Snippets

SELECT p.* 
FROM payment p
  JOIN transaction t
    ON t.payment_bk = p.bk
  JOIN payment_line pl
    ON  p.bk = pl.payment_bk
WHERE p.valid IS NULL
  AND p.valid_to IS NULL
  AND p.date>{ts '2015-05-21 00:00:00'} 
  AND p.date<{ts '2015-05-22 00:00:00'} 
  AND t.valid IS NULL
  AND t.valid_to IS NULL
  AND t.transaction_status = 'CLOSED'
  AND NOT EXISTS (
    SELECT 1 
    FROM transaction t 
    WHERE t.payment_bk = p.bk 
      AND t.transaction_status <> 'CLOSED'
      AND t.valid and t.valid_to IS NULL
  )
  AND pl.valid IS NULL
  AND pl.valid_to IS NULL
  AND pl.payment_line_status = 'CLOSED'
  AND NOT EXISTS (
    SELECT 1 
    FROM payment_line 
    WHERE pl.payment_bk = p.bk 
      AND pl.payment_line_status <> 'CLOSED'
      AND pl.valid IS NULL
      AND pl.valid_to IS NULL
  );
WHERE p.valid AND p.valid_to IS NULL

Context

StackExchange Code Review Q#91380, answer score: 4

Revisions (0)

No revisions yet.