-1

I've got a task to work with 4 different tables. I think I've got the "logic" correct, but I think I'm failing on joining the various separately working things together.

  • The Case somehow returns two rows when the comparison is true; if it isn;t, it displays (correctly) just one. Works fine without joins.
  • The count subquery works when by itself, but when I'm trying to tie it together, it displays anything from showing the same number everywhere or displaying far too large numbers (likely multiples or multiples).
Select Distinct RPD_PERSONS.PERSON_ID "id",
    RPD_PERSONS.SURN_TXT ||' '|| RPD_PERSONS.NAME_TXT "Name",
    Case ADD_ROLE_PERS.ROLE_CODE When 'Manager'
        Then 'yes'
        Else 'no'
        End "Manager",
    (
    Select Count(LDD_CERTS.Cert_ID)
    From LDD_CERTS
    Join LDD_PERS_CERTS
        On LDD_PERS_CERTS.CERT_ID = LDD_CERTS.CERT_ID
    Where MONTHS_BETWEEN(LDD_CERTS.VALID_TO,SYSDATE)>0
        And LDD_PERS_CERTS.CERT_CHANGE_TYPE>=0
    ) "no. of certificates"
From RPD_PERSONS
Join ADD_ROLE_PERS
    On ADD_ROLE_PERS.Person_ID = RPD_PERSONS.Person_ID
Where RPD_PERSONS.Partic_ID = 1
Group By RPD_PERSONS.PERSON_ID, RPD_PERSONS.SURN_TXT ||' '|| RPD_PERSONS.NAME_TXT, ADD_ROLE_PERS.ROLE_CODE
Order By RPD_PERSONS.Person_ID;

This is the subquery that, by itself, seems to work perfectly.

Select LDD_PERS_CERTS.PERSON_UID,Count(LDD_CERTS.Cert_ID)
From LDD_CERTS
Join LDD_PERS_CERTS
    ON LDD_PERS_CERTS.CERT_ID = LDD_CERTS.CERT_ID
Where MONTHS_BETWEEN(LDD_CERTS.VALID_TO,SYSDATE)>0
    AND LDD_PERS_CERTS.CERT_CHANGE_TYPE>=0
Group By LDD_PERS_CERTS.PERSON_UID
order by LDD_PERS_CERTS.PERSON_UID;
philipxy
  • 14,867
  • 6
  • 39
  • 83
Ghnol
  • 3
  • 1
  • Please in code questions give a [mre]. [ask] [Help] When you get a result you don't expect, pause your overall goal, chop to the 1st subexpression with unexpected result & say what you expected & why, justified by documentation. (Debugging fundamental.) Please before considering posting read the manual/reference & google any error message & many clear, concise & precise phrasings of your question/problem/goal, with & without your particular names/strings/numbers, 'site:stackoverflow.com' & tags; read many answers. Reflect research in posts. – philipxy May 10 '22 at 03:05
  • Duplicate of [Two SQL LEFT JOINS produce incorrect result](https://stackoverflow.com/q/12464037/3404097) This seems to be a common error where people want some joins, each possibly involving a different key, of some subqueries, each possibly involving join and/or aggregation, but they erroneously try to do all the joining then all the aggregating or to aggregate over previous aggregations. Write separate aggregate subqueries over appropriate rows and/or aggregate (maybe distinct) case statements picking rows of (possibly join result) tables; then join the subqueries together. (A faq.) – philipxy May 10 '22 at 03:06

2 Answers2

0

You have a lot of things going on although a short query to get it, but let me try to summarize what I THINK you are trying to get.

You want a list of distinct people within the company with a count of how many ACTIVE certs (not expired) per person. From that, you also want to know if they are in a management position or not (via roles).

Q: For a person who may be a manager, but also an under-manager to a higher-up, do you want to see that person in both roles as typical business structures could have multiple layers of management, OR... Do you only care to see a person once, and if they are a manager OR some other level. What if a person has 3 or more roles, do you want to see them every instance? If your PRIMARY care is Manager Yes or No, the query gets even more simplified.

Now, your query of counts for valid certs. The MONTHS_BETWEEN() function appears to be you are running in Oracle. Based on the two parameters of the Valid_To date compared to sysdate is an indication that the valid to is always intended to be in the future (ie: Still has an active cert). If this is the case, you will not be able to optimize query as function calling is not Sargable Instead, you should only have to do where Valid_To > SysDate, in other words, only those that have not yet expired. You MIGHT even be better served by pre-aggregating all counts of still active cert counts per Cert ID, then joining to the person certs table since the person cert check is for all where the cert_change_type >= 0 which could imply ALL. What condition would a Cert_Change_Type be anything less than zero, and if never, that where clause is pointless.

Next, your SELECT DISTINCT query needs a bit of adjustments. Your column-based select has no context to the outer person ID and is just aggregating the total certs. There is no correlation to the person ID to the certs being counted for. I can only GUESS that there is some relationship such as

RPD_Persons.Person_id =  LDD_Pers_Certs.Person_UID

Having stated all that, I would have the following table/indexes

table              index
LDD_PERS_CERTS     ( CERT_CHANGE_TYPE, PERSON_UID, CERT_ID )
LDD_CERTS          ( valid_to, cert_id )
RPD_PERSONS        ( partic_id, person_id, surn_txt, name_txt )
ADD_ROLE_PERS      ( person_id, role_code )

I would try something like

Select 
        lpc.PERSON_UID,
        ValCerts.CertCount
    From 
        ( select
                Cert_id,
                count(*) CertCounts
            from
                LDD_CERTS
            where
                Valid_To > sysDate
            group by
                Cert_id ) ValCerts
            JOIN LDD_PERS_CERTS lpc
                on ValCerts.Cert_id = lpc.cert_id
    Where 
        lpc.CERT_CHANGE_TYPE >= 0

Now, if you only care if a given person is a manager or not, I would pre-query that only as you were not actually returning a person's SPECIFIC ROLE, just the fact they were a manager or not. My final query might look like'

select 
        p.PERSON_ID id,
        max( p.SURN_TXT || ' ' || p.NAME_TXT ) Name,
        max( Case when arp.Person_id IS NULL
                then 'no' else 'yes' end ) Manager,
        max( coalesce( certs.CertCount, 0 )) ActiveCertsForUser
    from
        RPD_PERSONS p
            LEFT Join ADD_ROLE_PERS arp
                On p.Person_ID = arp.Person_ID
                AND arp.role_code = 'Manager'
            LEFT JOIN
            (   Select 
                        lpc.PERSON_UID,
                        ValCerts.CertCount
                    From 
                        ( select
                                Cert_id,
                                count(*) CertCounts
                            from
                                LDD_CERTS
                            where
                                Valid_To > sysDate
                            group by
                                Cert_id ) ValCerts
                            JOIN LDD_PERS_CERTS lpc
                                on ValCerts.Cert_id = lpc.cert_id
                                AND lpc.CERT_CHANGE_TYPE >= 0 )
            ) Certs
                on p.Person_id = Certs.Person_uid
    Where 
        p.Partic_ID = 1
    GROUP BY
        p.PERSON_ID

Now, if the p.partic_id = 1 represents only 1 person, then that wont make as much sense to query all people with a given certificate status, etc. But if Partic_id = 1 represents a group of people such as within a given association / division of a company, then it should be fine.

Any questions, let me know and I can revise / update answer

DRapp
  • 47,638
  • 12
  • 72
  • 142
  • Rereading my original question made me very surprised how much you've been able to guess. Your solution is almost 100% perfect. as @NickW made me realize, the arp.role_code has multiple entries of "roles" for each id. That, I think, gave me double rows on my CASE - the query found 'Manager', which gave the 'Yes', but also others, that gave 'No'. I guess this leads to another subquery? All I need is a Yes if one of the roles for the ID is Manager, if none is found -> No. and finally, yes, the p.Partic_ID = 1 represents group or a company. I need to write better questions. – Ghnol May 09 '22 at 13:07
  • @Ghnol, glad this answer worked. However, adjusted via LEFT-JOIN and aggregating to allow both manager and non-manager entries, but getting the MAX() so those that may be upper management, yet also a subordinate get shown in result. My original query of a join only on the roles of managers would have only returned managers, not both. – DRapp May 10 '22 at 11:46
0
  • CASE issue: there can be, presumably, be multiple records in ADD_ROLE_PERS for each person. If a person can have two or more roles running concurrently then you need to decide what the business logic is that you need to use to handle this. If a person can only have one active role at a time presumably there is a "active/disabled" column or effective date columns you should be using to identify the active record (or, potentially, there is a data issue).
  • The subquery should return the same value for every single row in your resultset, as it is completely isolated/standalone from your main query. If you want it to produce counts that are relevant to each row then you will need to connect it to the tables in the main table (look up correlated subqueries if you don't know how to so this)
NickW
  • 8,430
  • 2
  • 6
  • 19