6
\$\begingroup\$

Here we trying to fix logs that have overlaps with each other. Am I doing it right? Is there any way to improve/refactor it?

 DROP TYPE IF EXISTS logs_with_error;
  CREATE TYPE logs_with_error AS (
      model_id INT,
      model_type TEXT,
      initiator_id INT,
      initiator_type TEXT,
      error_type_id INT,
      state INT
  );

  CREATE OR REPLACE FUNCTION normalize_moderation_log_overflow() RETURNS INTEGER AS
      $BODY$
  DECLARE
      logs logs_with_error%rowtype;
      model moderation_logs%rowtype;
      initiator moderation_logs%rowtype;
  BEGIN
      FOR logs IN SELECT model_id, model_type, initiator_id, initiator_type, error_type_id, state
    FROM errors
    WHERE state = #{ Error.states[:initial] } 
    AND error_type_id = (SELECT id FROM error_types WHERE normalizer_type = 'moderation_log_overflow')
      LOOP
        SELECT * INTO model FROM moderation_logs WHERE id = logs.model_id;
        SELECT * INTO initiator FROM moderation_logs WHERE id = logs.initiator_id;

        IF model.curr_state = initiator.curr_state AND model.next_state = initiator.next_state THEN
          IF model.ended_at < initiator.ended_at THEN
            UPDATE moderation_logs SET ended_at = initiator.ended_at WHERE id = model.id;
          END IF;
          DELETE FROM moderation_logs WHERE id = initiator.id;
        ELSE
          UPDATE moderation_logs SET started_at = model.ended_at WHERE id = initiator.id;
        END IF;

        UPDATE errors 
        SET state = #{ Error.states[:resolved] };

        RAISE NOTICE '%', logs;
      END LOOP;
      RETURN 1;
  END
  $BODY$
  LANGUAGE 'plpgsql' ;
\$\endgroup\$
  • 3
    \$\begingroup\$ Are you working with very large (tens of thousands or more) record sets or relatively small? \$\endgroup\$ – Phrancis Jul 9 '15 at 19:15
  • \$\begingroup\$ Yes, with very large sets. \$\endgroup\$ – Petr Guskov Jul 22 '16 at 12:51
4
\$\begingroup\$

There's really only rather small things I'd change / adapt


CREATE OR REPLACE FUNCTION normalize_moderation_log_overflow() RETURNS INTEGER AS

Since you always return 1, this function actually should be a void.

CREATE OR REPLACE FUNCTION normalize_moderation_log_oveflow () RETURNS VOID AS

Your Indentation seems off a little:

BEGIN
    FOR logs ...
  FROM ...

I'd prefer to indent the SELECT statement in reference to the FOR .. LOOP like this:

BEGIN
  FOR logs IN SELECT ...
      FROM errors

AND error_type_id = (SELECT id FROM error_types WHERE normalizer_type = 'moderation_log_overflow')

This can (and probably should be) extracted into a constant

log_overflow CONSTANT INT := SELECT id FROM error_types WHERE normalizer_type = 'moderation_log_overflow' LIMIT 1;

Which makes the condition look like this:

AND error_type_id = log_overflow

    UPDATE errors 
    SET state = #{ Error.states[:resolved] };

All other DML statements you have are in a single line, why not this one, too?

UPDATE errors SET state = #{ Error.states[:resolved] };
| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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