Allon Mureinik has posted comments on this change.

Change subject: core:  Custom Materialized Views should be...
......................................................................


Patch Set 5: Looks good to me, but someone else must approve

(2 inline comments)

See inline

....................................................
File backend/manager/dbscripts/materialized_views_sp.sql
Line 383:     RETURN;
Line 384:  END; $procedure$
Line 385:  LANGUAGE plpgsql;
Line 386: 
Line 387: CREATE OR REPLACE FUNCTION ActivateMaterializedView(v_matview NAME, 
v_active BOOLEAN)
the semantics of calling ActivateMaterializedView("my_mv", false) to deactivate 
an MV sound a bit odd to me.

How about having something like ChangeMateriazliedViewStatus(boolean 
v_activate), and wrapping it with activate and deactivate functions?
Line 388:  RETURNS VOID
Line 389: AS $procedure$
Line 390: DECLARE
Line 391:      v_entry materialized_views%ROWTYPE;


....................................................
Commit Message
Line 24:           Defining min refresh flood time in seconds (in the same
Line 25:           manner log flood is handled)
Line 26: 
Line 27: Change-Id: I6a6857a1691c1dafa1a7edcd58b292e70c78b318
Line 28: Signed-off-by: Eli Mesika <emes...@redhat.com>
You don't need two of these :-)
Line 29: Bug-Url: https://bugzilla.redhat.com/show_bug.cgi?id=907232


--
To view, visit http://gerrit.ovirt.org/11669
To unsubscribe, visit http://gerrit.ovirt.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6a6857a1691c1dafa1a7edcd58b292e70c78b318
Gerrit-PatchSet: 5
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Allon Mureinik <amure...@redhat.com>
Gerrit-Reviewer: Eli Mesika <emes...@redhat.com>
Gerrit-Reviewer: Yair Zaslavsky <yzasl...@redhat.com>
_______________________________________________
Engine-patches mailing list
Engine-patches@ovirt.org
http://lists.ovirt.org/mailman/listinfo/engine-patches

Reply via email to