Martin Peřina has posted comments on this change. Change subject: core: Add ExternalVariable entity ......................................................................
Patch Set 2: (3 comments) http://gerrit.ovirt.org/#/c/27934/2/packaging/dbscripts/external_variable_sp.sql File packaging/dbscripts/external_variable_sp.sql: Line 5: -- UpsertExternalVariable is used in fence_kdump listener Line 6: CREATE OR REPLACE FUNCTION UpsertExternalVariable( Line 7: v_var_name VARCHAR(50), Line 8: v_var_value VARCHAR(255), Line 9: v_updated TIMESTAMP WITH TIME ZONE) > This should not be passed as parameter, rather it should be set from LOCALT Do you think, that we will never need to pass predefined timestamp (I mean timestamp set by logic and not by db)? Line 10: RETURNS VOID Line 11: AS $procedure$ Line 12: DECLARE Line 13: db_updated TIMESTAMP WITH TIME ZONE; Line 13: db_updated TIMESTAMP WITH TIME ZONE; Line 14: BEGIN Line 15: -- set updated to db now if not provided Line 16: db_updated := v_updated; Line 17: IF db_updated IS NULL THEN > Redundant IF block , see my 1st comment on that Done Line 18: SELECT NOW() INTO db_updated; Line 19: END IF; Line 20: Line 21: UPDATE external_variable Line 20: Line 21: UPDATE external_variable Line 22: SET Line 23: var_value = v_var_value, Line 24: updated = db_updated > Please rename updated=> _update_date like other tables of engine Done Line 25: WHERE var_name = v_var_name; Line 26: Line 27: IF NOT found THEN Line 28: INSERT INTO external_variable( -- To view, visit http://gerrit.ovirt.org/27934 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I61ffd646c102c01c34478309d072f74a8f7d7edf Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Barak Azulay <bazu...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Martin Peřina <mper...@redhat.com> Gerrit-Reviewer: Oved Ourfali <oourf...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches
