3
\$\begingroup\$

IBM's Maximo Asset Management platform has a WORKORDER table (265 columns).

Details:

The WORKORDER table contains two different kinds of rows:

  • WO records (istask = 0)
  • Task records (istask = 1).

The table has columns that store the actual costs (populated for both non-task WOs and for tasks):

  • actlabcost (actual labor cost)
  • actmatcost (actual material cost)
  • actservcost (actual services cost)
  • acttoolcost (actual tool costs)

View:

I've written a query/view that selects non-task WOs.

For those non-task workorders, the view rolls up the costs from the related tasks and summarizes them in these columns:

  • actlabcost_incltask
  • actmatcost_incltask
  • actservcost_incltask
  • acttoolcost_incltask
  • acttotalcost_incltask

I plan to use the view for multiple reports. So I've included all 265 columns in the view via select * (although, Oracle will convert the select * to actual column names when the view is created).

--WO_INCL_TASK_ACT_VW (non-task WOs, includes task actuals) select t.task_actlabcost, t.task_actmatcost, t.task_actservcost, t.task_acttoolcost, t.task_acttotalcost, nt.actlabcost + t.task_actlabcost as actlabcost_incltask, nt.actmatcost + t.task_actmatcost as actmatcost_incltask, nt.actservcost + t.task_actservcost as actservcost_incltask, nt.acttoolcost + t.task_acttoolcost as acttoolcost_incltask, t.task_acttotalcost + nt.actlabcost + nt.actmatcost + nt.actservcost + nt.acttoolcost as acttotalcost_incltask, nt.* from workorder nt --non-task WOs left join ( select parent, sum(actlabcost) as task_actlabcost, sum(actmatcost) as task_actmatcost, sum(actservcost) as task_actservcost, sum(acttoolcost) as task_acttoolcost, sum(actlabcost) + sum(actmatcost) + sum(actservcost) + sum(acttoolcost) as task_acttotalcost from workorder group by parent, istask having istask = 1 ) t --tasks on nt.wonum = t.parent where nt.istask = 0

Question:

The view works just fine. However, it's fairly lengthy for what it does.

Can it be improved?

\$\endgroup\$
4
  • \$\begingroup\$ Are these columns - sum(actlabcost) + sum(actmatcost) + sum(actservcost) + sum(acttoolcost) as task_acttotalcost - nullable? \$\endgroup\$ Commented Nov 8, 2020 at 0:41
  • \$\begingroup\$ @Reinderien I did a search on the table for null values. I couldn't find any. So I think the application must be preventing nulls and storing zeros instead. i.sstatic.net/e9As1.png. If those columns were nullable, would it compromise my query? \$\endgroup\$ Commented Nov 8, 2020 at 0:59
  • \$\begingroup\$ @Reinderien I looked at the application too. It seems to default to zero, not null. i.sstatic.net/iPiwW.png \$\endgroup\$ Commented Nov 8, 2020 at 1:04
  • 1
    \$\begingroup\$ would it compromise my query? No, but it would affect my suggestion. \$\endgroup\$ Commented Nov 8, 2020 at 1:04

3 Answers 3

Reset to default
3
\$\begingroup\$

I know you say that

I plan to use the view for multiple reports. So I've included all 265 columns in the view via select *

but given the truly absurd column count in that table, I would consider a select-splat to be a very last resort. Are you able to narrow this at all?

Generally your query seems sane. Since you say that the following columns are non-nullable,

sum(actlabcost) + sum(actmatcost) + sum(actservcost) + sum(acttoolcost)

should be equivalent to

sum(actlabcost + actmatcost + actservcost + acttoolcost)
\$\endgroup\$
6
  • \$\begingroup\$ Right. Regarding the second option: summing a null instead of a zero would result in a null, which is not what I want. \$\endgroup\$ Commented Nov 8, 2020 at 1:24
  • \$\begingroup\$ Right; but you told me that there aren't any so it shouldn't matter. \$\endgroup\$ Commented Nov 8, 2020 at 13:41
  • \$\begingroup\$ If a workorder doesn't have any tasks that are joined to it, then I think nt.actlabcost+t.task_actlabcost would result in null, even if nt.actlabcost wasn't null. This would not be the desired result (1 + null = null). \$\endgroup\$ Commented Nov 19, 2020 at 21:51
  • \$\begingroup\$ task_acttotalcost is based on columns all from the same table, workorder, right? So they'll be all-null or all-non-null. No problem in using normal addition there. The trouble comes when you add task_acttotalcost to find acttotalcost_incltask, which has the null problem in your original post and needs a coalesce to zero. \$\endgroup\$ Commented Nov 19, 2020 at 22:12
  • \$\begingroup\$ I think I understand your point, but my concern is when there aren't any tasks associated with (aka joined to) the non-task workorders. So, nt.actlabcost+t.task_actlabcost could end up being 1243.00 + null = null. \$\endgroup\$ Commented Nov 19, 2020 at 22:30
1
\$\begingroup\$

It looks like you are trying to get the cost of a work order including its children, if it has any, by joining from workorder nt to your in-line view t where nt.wonum = t.parent.

Are you aware that there is a wogroup column on workorder whose value is the same as wonum for non-task work orders and the same as parent on task work orders? So, you could remove the subquery from your query and just group your workorder records by wogroup.

For example, to get the actual labor cost for this work order and its children, instead of doing this:

select ... nt.actlabcost + t.task_actlabcost as actlabcost_incltask, ... nt.* from workorder nt --non-task WOs left join ... ) t --tasks on nt.wonum = t.parent where nt.istask = 0

you could do this:

select sum(actlabcost) as acttotalcost, wogroup from workorder where woclass in ('WORKORDER', 'ACTIVITY') and siteid = 'SERVICES' group by wogroup

On a separate point, in your in-line view, is there a good reason for having istask = 1 instead of where istask = 1. In my experience, the having clause is used for conditions that use aggregate functions, and your "flat" istask = 1 seems out of place there.

\$\endgroup\$
3
  • 1
    \$\begingroup\$ Hi. Welcome to Code Review! This is an alternative code. Please note that we emphasize the review portion of our name. And this doesn't provide an explicit observation about the existing code. Implicitly you seem to be suggesting that the existing code is more complicated than it needs to be. Please say that explicitly and quote the code that you are simplifying. As is, it's hard to see how this relates to the original code. \$\endgroup\$ Commented Sep 20, 2021 at 6:27
  • \$\begingroup\$ Good call. I should have used WOGROUP. Much simpler and faster. \$\endgroup\$ Commented Sep 20, 2021 at 12:20
  • \$\begingroup\$ Related post: Group by x, get other fields too. \$\endgroup\$ Commented Sep 21, 2021 at 23:05
0
\$\begingroup\$

This is the query I ended up going with. I believe it handles nulls correctly.

Note: The cost columns in the WORKORDER table are not nullable (actlabcost, actmatcost, actservcost, acttoolcost).

So that helps.

select nt.wonum, nt.parent, nt.hierarchypath, nt.classstructureid, nt.division, nt.worktype, nt.status, nt.glaccount, nt.fircode, actstart, actfinish, nt.siteid, nvl(nt.actlabcost,0) + nvl(t.t_actlabcost,0) as actlabcost, nvl(nt.actmatcost,0) + nvl(t.t_actmatcost,0) as actmatcost, nvl(nt.actservcost,0) + nvl(t.t_actservcost,0) as actservcost, nvl(nt.acttoolcost,0) + nvl(t.t_acttoolcost,0) as acttoolcost, nvl(t.t_acttotalcost,0) + nvl(nt.actlabcost,0) + nvl(nt.actmatcost,0) + nvl(nt.actservcost,0) + nvl(nt.acttoolcost,0) as acttotalcost, coalesce(nt.parent, nt.wonum) as parent_group --only works when WO hierarchy is 1 or 2 levels (not more) from workorder nt --non-task WOs left join ( select parent, sum(actlabcost) as t_actlabcost, sum(actmatcost) as t_actmatcost, sum(actservcost) as t_actservcost, sum(acttoolcost) as t_acttoolcost, sum(actlabcost + actmatcost + actservcost + acttoolcost) as t_acttotalcost from workorder where istask = 1 and woclass in ('WORKORDER', 'ACTIVITY') and siteid = 'SERVICES' group by parent ) t --tasks on nt.wonum = t.parent where nt.istask = 0 and woclass in ('WORKORDER', 'ACTIVITY') and siteid = 'SERVICES'
\$\endgroup\$
1
  • 2
    \$\begingroup\$ It's time to get out of the habit of using NVL() and use COALESCE() instead. NVL() can implicitly convert to a character, isn't in the SQL standard, and doesn't perform short-circuiting. COALESCE() fixes all these problems. \$\endgroup\$ Commented Jan 23, 2021 at 17:32

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.