Is this PL/SQL Stored proc optimal?

Question

create or replace procedure upsertTrackingItemMetric(
 p_template_id number,
 p_jobDef varchar2,
 p_metric_type varchar2,
 p_metric_value varchar2
)
is
 PRAGMA AUTONOMOUS_TRANSACTION;
 sig_att_id number := null;
 sig_grp_id number := null;
 tims_id number := null;
 const_error_metric_id NUMBER := 3;
BEGIN
 select max(tims.ID) into tims_id
 from SREAPP.TRACKING_ITEM_METRIC_STORE tims
 join SREAPP.TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE sig_attrib
 on tims.ID = sig_attrib.TRACKING_ITEM_METRIC_STORE_ID
 join SREAPP.ESSLOG_ERR_SIGNATURE sig on sig_attrib.value = to_char(sig.SIGNATURE_GROUPING_ID)
 where tims.signature_id = 7
 and tims.metric_id = 3
 and sig_attrib.signature_attribute_id = 13
 and sig.TEMPLATE_ID = p_template_id
 and tims.TRACKING_ITEM = p_metric_type
 and sig.JOB_DEFINITION = p_jobDef fetch first row only;
 if tims_id is null then
 insert into TRACKING_ITEM_METRIC_STORE (TRACKING_ITEM, METRIC_ID, SIGNATURE_ID, VALUE, ACTIVE)
 values(p_metric_type, const_error_metric_id, 7, p_metric_value,1)
 returning ID into tims_id;
 else
 UPDATE TRACKING_ITEM_METRIC_STORE SET VALUE = p_metric_value, ACTIVE = 1 where ID = tims_id;
 end if;
 select max(SIGNATURE_GROUPING_ID) into sig_grp_id
 from ESSLOG_ERR_SIGNATURE
 where JOB_DEFINITION = p_jobDef and TEMPLATE_ID = p_template_id fetch first row only;
 if sig_grp_id is null then
 -- first create the group
 INSERT INTO ESSLOG_ERR_SIGNATURE_GROUP (NAME, DESCRIPTION)
 values(
 'create by ' || user || ' on ' || sysdate,
 p_template_id || '~' || p_jobDef
 )
 returning ID into sig_grp_id;
 -- Now tell the signature which group it belongs to
 insert into ESSLOG_ERR_SIGNATURE (SIGNATURE_GROUPING_ID,JOB_DEFINITION,TEMPLATE_ID)
 values (sig_grp_id,p_jobDef,p_template_id);
 end if;
 select max(SIGNATURE_ATTRIBUTE_ID) into sig_att_id from TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE
 where TRACKING_ITEM_METRIC_STORE_ID = tims_id
 and SIGNATURE_ATTRIBUTE_ID = 13
 and VALUE = to_char(sig_grp_id) fetch first row only;
 -- we only need to insert something if we had to create a group
 if sig_att_id is null then
 insert into TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE (TRACKING_ITEM_METRIC_STORE_ID, SIGNATURE_ATTRIBUTE_ID, VALUE)
 VALUES (tims_id, 13, to_char(sig_grp_id)) returning SIGNATURE_ATTRIBUTE_ID into sig_att_id;
 end if;
 commit;
END;

For example, I am taking advantage of the fact that max returns null on an empty set result.

• Reinderien: Those hard-coded IDs (7, 3...) are a severe smell. Where do they come from? Why those IDs specifically?
• Reinderien: In sane RDBMS this has direct upsert support via on conflict. In Oracle, there's UPSERT.
• Reinderien: In theory MERGE is a better way to go because it's SQL-standard. Oracle also supports this.
• Christian Bongiorno: @Reinderien - upsert isn't supported on 19.x - sorry, I should have put that in the question
• Christian Bongiorno: Yeah, they are from another table I didn't come up with - they're FKs without being FK constraints. My hands are tied there

Answer 1

As mentioned, consider MERGE to handle the conditional INSERT and UPDATE actions. Doing so, code arguably reads easier with three SQL statements and you avoid the procedural IF/THEN/ELSE statements and inserted variables. Because you do use some inserted variables in subsequent statements, you would need CTEs in the last merge which even necessitates a group by clause.

Additionally, consider a few other items:

• avoid mix cases such as camel casing in any object name including procedures;
• use a trigger for any action resulting from INSERT which avoids manually doing so in a separate procedure;
• not necessary but may help in readability, move some WHERE conditions to ON clauses especially with all inner joins;
• for aggregate queries without GROUP BY, fetch first row only may not be necessary;
• avoid hard-coded values for named variables to help in readability like the one declared constant you use since such constants may change in future as parameters.

create or replace procedure UPSERT_TRACKING_ITEM_METRIC(
 p_template_id number,
 p_jobDef varchar2,
 p_metric_type varchar2,
 p_metric_value varchar2
)
is
 PRAGMA AUTONOMOUS_TRANSACTION;
 sig_att_id number := null;
 sig_grp_id number := null;
 tims_id number := null;
 const_error_metric_id NUMBER := 3;
BEGIN
 -- FIRST MERGE
 merge into TRACKING_ITEM_METRIC_STORE tgt
 using (
 select max(tims.ID) as max_tims_id
 from SREAPP.TRACKING_ITEM_METRIC_STORE tims
 join SREAPP.TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE sig_attrib
 on tims.ID = sig_attrib.TRACKING_ITEM_METRIC_STORE_ID
 and tims.signature_id = 7
 and tims.metric_id = 3
 and tims.TRACKING_ITEM = p_metric_type
 join SREAPP.ESSLOG_ERR_SIGNATURE sig 
 on sig_attrib.value = to_char(sig.SIGNATURE_GROUPING_ID) 
 and sig_attrib.signature_attribute_id = 13
 and sig.TEMPLATE_ID = p_template_id 
 and sig.JOB_DEFINITION = p_jobDef
 ) src
 on (src.max_tims_id = tgt.ID)
 when matched then
 update set tgt."VALUE" = p_metric_value, tgt.ACTIVE = 1 
 where tgt.ID = src.max_tims_id;
 when not matched then
 insert (tgt.TRACKING_ITEM, tgt.METRIC_ID, tgt.SIGNATURE_ID, tgt."VALUE", tgt.ACTIVE)
 values (p_metric_type, const_error_metric_id, 7, p_metric_value, 1);
 
 -- SECOND MERGE
 merge into ESSLOG_ERR_SIGNATURE_GROUP tgt
 using (
 select max(SIGNATURE_GROUPING_ID) as max_sig_grp_id
 from ESSLOG_ERR_SIGNATURE s
 where s.JOB_DEFINITION = p_jobDef 
 and s.TEMPLATE_ID = p_template_id
 ) src
 on (src.max_sig_grp_id = tgt.ID)
 when not matched
 insert (tgt."NAME", tgt.DESCRIPTION)
 values (
 'create by ' || user || ' on ' || sysdate,
 p_template_id || '~' || p_jobDef
 );
 -- THIRD MERGE
 merge into TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE tgt
 using (
 with sub_t as (
 select max(tims.ID) as max_tims_id
 from SREAPP.TRACKING_ITEM_METRIC_STORE tims
 join SREAPP.TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE sig_attrib
 on tims.ID = sig_attrib.TRACKING_ITEM_METRIC_STORE_ID
 and tims.signature_id = 7
 and tims.metric_id = 3
 and tims.TRACKING_ITEM = p_metric_type
 join SREAPP.ESSLOG_ERR_SIGNATURE sig 
 on sig_attrib.value = to_char(sig.SIGNATURE_GROUPING_ID) 
 and sig_attrib.signature_attribute_id = 13
 and sig.TEMPLATE_ID = p_template_id 
 and sig.JOB_DEFINITION = p_jobDef
 ), sub_e as (
 select max(SIGNATURE_GROUPING_ID) as max_sig_grp_id
 from ESSLOG_ERR_SIGNATURE s
 where s.JOB_DEFINITION = p_jobDef 
 and s.TEMPLATE_ID = p_template_id
 )
 select sub_t.max_tims_id, 
 to_char(sub_e.max_sig_grp_id) as max_sig_grp_id_char,
 max(main.SIGNATURE_ATTRIBUTE_ID) as max_sig_att_id 
 from TRACKING_ITEM_METRIC_SIGNATURE_ATTRIBUTE main
 join sub_t
 on main.TRACKING_ITEM_METRIC_STORE_ID = sub_t.max_tims_id
 and main.SIGNATURE_ATTRIBUTE_ID = 13
 join sub_e
 on main."VALUE" = to_char(sub_e.max_sig_grp_id)
 group by sub_t.max_tims_id,
 to_char(sub_e.max_sig_grp_id)
 ) src
 on (
 src.max_sig_att_id = tgt.SIGNATURE_ATTRIBUTE_ID and 
 src.max_tims_id = tgt.TRACKING_ITEM_METRIC_STORE_ID and
 src.max_sig_grp_id_char = tgt."VALUE"
 )
 when not matched
 insert (tgt.TRACKING_ITEM_METRIC_STORE_ID, tgt.SIGNATURE_ATTRIBUTE_ID, tgt."VALUE")
 values (src.max_tims_id, 13, src.max_sig_grp_id_char);
 commit;
END;

Proposed Trigger:

create or replace trigger ESSLOG_ERR_SIGNATURE_GROUP_TRIGGER
 AFTER INSERT
 ON ESSLOG_ERR_SIGNATURE_GROUP
 FOR EACH ROW 
DECLARE
 p_template_id number;
 p_jobDef varchar2;
BEGIN
 -- SPLIT DESCRIPTION COLUMN BY ~ DELIMITER
 p_template_id := substr(:new.DESCRIPTION, 1, instr(:new.DESCRIPTION, '~', -1, 1) -1);
 p_jobDef := substr(:new.DESCRIPTION, instr(:new.DESCRIPTION, '~', -1, 1) +1);
 -- INSERT INTO RELATED TABLE 
 insert into ESSLOG_ERR_SIGNATURE (SIGNATURE_GROUPING_ID, JOB_DEFINITION, TEMPLATE_ID)
 values (:new.ID, p_jobDef, p_template_id);
END;

Note: Above refactoring has not been tested on actual data. Please take the time to solve issues and not expect a seamless answer. Slight adjustments may be needed.