Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I have an unstable PL/pgSQL function:

DECLARE
l RECORD;
events_for_machine integer;
before_event "PRD".events_log;
machines_ids integer[];
island_controller RECORD;
before_order "PRD".events_log;
before_detail "PRD".events_log;
before_pallete "PRD".events_log;
before_operation "PRD".events_log;
timer timestamp;
timer2 timestamp;

BEGIN
machines_ids = string_to_array(machines_ids_g,',')::integer[];
for l in 
select m.*
from
    "PRD".machines m
    inner join
    unnest(machines_ids) n(id) on n.id = m.id
where
    m.start_work_date < begin_date_g
order by m.id
LOOP

SELECT * INTO island_controller FROM "STRUCT".island_machines WHERE machine_id=l.id;

RAISE NOTICE 'pobieram zdarzenie before dla maszyny %',l.id;

    SELECT * INTO before_event FROM "PRD".events_log WHERE plc_time < begin_date_g AND (((event_type_id IN (1,51) AND machine_id = island_controller.controller_id AND island_id = island_controller.island_id))
    OR (event_type_id IN (2000,2001) AND machine_id=l.id)) ORDER BY plc_time DESC LIMIT 1;

IF before_event.plc_time IS NOT NULL THEN    

RAISE NOTICE 'Getting info about first machine work time struct element';
RETURN QUERY SELECT * FROM "PRD".events_log WHERE event_type_id = 113 AND machine_id=l.id AND plc_time < before_event.plc_time ORDER BY plc_time DESC LIMIT 1;
RETURN QUERY SELECT * FROM "PRD".events_log WHERE event_type_id = 102 AND machine_id=l.id AND plc_time < before_event.plc_time ORDER BY plc_time DESC LIMIT 1;
RETURN QUERY SELECT * FROM "PRD".events_log WHERE event_type_id = 111 AND machine_id=l.id AND plc_time < before_event.plc_time ORDER BY plc_time DESC LIMIT 1;
RETURN QUERY SELECT * FROM "PRD".events_log WHERE event_type_id = 1010 AND machine_id=l.id AND plc_time < before_event.plc_time ORDER BY plc_time DESC LIMIT 1;
RETURN NEXT before_event;


END IF;
RAISE NOTICE 'generuje zdarzenia wlasciwe dla maszyny %',l.id;
    RETURN QUERY SELECT * FROM "PRD".events_log WHERE 
            (event_type_id = ANY ('{1,51}'::integer[]) AND (machine_id=island_controller.controller_id AND island_id = island_controller.island_id) AND (plc_time BETWEEN begin_date_g AND end_date_g))
            OR (event_type_id = ANY ('{2000,2001,107}'::integer[]) AND machine_id=l.id AND (plc_time >= begin_date_g AND plc_time <= end_date_g))
            OR ((event_type_id = ANY ('{101,102,103,301,1010}'::integer[]) OR ((event_type_id >= 5000) AND (event_type_id <= 5999))) AND machine_id=l.id AND plc_time >= begin_date_g AND plc_time <= end_date_g) ORDER BY plc_time;

RAISE NOTICE 'koniec dla maszyny %',l.id;
END LOOP;

END;

Sometimes the function execution time is ~9 seconds and sometimes ~40 seconds for the same arguments.

What does it depend on? What could be so inefficient?

share|improve this question
    
Parameter definition and other details in the header are essential. Always post a complete function - will help your chances on a response. –  Erwin Brandstetter Mar 11 '13 at 7:52
add comment

1 Answer 1

I find this query difficult to read to begin with. Here are my comments:

  1. When you DECLARE a bunch of variables it is useful to name them such that they will be easy for another programmer to identify throughout the script. For example adding v_ in front of them is a good way. Also calling a variable just l is pretty cryptic.

  2. select * is generally not a good idea. I see you use that a lot and it may be part of your performance issues. Part of why it may be problematic is also that your result set is unpredictable, if a column is added or dropped for instance, or renamed, your query would not be repeatable. It's better to get a "column does not exist" error than get unpredictable result set. Be explicit.

  3. The lack of proper indenting in your script, as well as the lack of a proper explanation in your question (or as comments within your script), in addition to some of it being in Polish it seems, makes it very difficult to decrypt what you are doing here unless you provide more information.

  4. You have lots of variables, most of them holding lots of data (select * into etc.). This likely is affecting performance. Multiple levels of nested subqueries can also slow it down.

share|improve this answer
add comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.