Eli Mesika has posted comments on this change.

Change subject: core: Add VdsKdumpStatus entity
......................................................................


Patch Set 12:

(8 comments)

http://gerrit.ovirt.org/#/c/27200/12/packaging/dbscripts/upgrade/03_05_0500_add_vds_kdump_status.sql
File packaging/dbscripts/upgrade/03_05_0500_add_vds_kdump_status.sql:

Line 1: CREATE TABLE vds_kdump_status (
Line 2:     vds_id UUID NOT NULL,
Line 3:     status VARCHAR(20) NOT NULL,
Line 4:     address VARCHAR(50) NOT NULL,
Can this be a full FQDN and 50 chars may be not enough for that ???
Why not defining that as the ip in vds_static which is character varying(255)
Line 5:     CONSTRAINT pk_vds_kdump_status PRIMARY KEY(vds_id)
Line 6: ) WITH OIDS;
Line 7: 
Line 8: ALTER TABLE ONLY vds_kdump_status


Line 4:     address VARCHAR(50) NOT NULL,
Line 5:     CONSTRAINT pk_vds_kdump_status PRIMARY KEY(vds_id)
Line 6: ) WITH OIDS;
Line 7: 
Line 8: ALTER TABLE ONLY vds_kdump_status
This can be done as part of the CREATE TABLE statement
Line 9:     ADD CONSTRAINT fk_vds_kdump_status_vds_static
Line 10:     FOREIGN KEY (vds_id)


Line 8: ALTER TABLE ONLY vds_kdump_status
Line 9:     ADD CONSTRAINT fk_vds_kdump_status_vds_static
Line 10:     FOREIGN KEY (vds_id)
Line 11:     REFERENCES vds_static(vds_id) ON DELETE CASCADE;
Line 12
Due to your Get* SP expect also an index on status


http://gerrit.ovirt.org/#/c/27200/12/packaging/dbscripts/vds_kdump_status_sp.sql
File packaging/dbscripts/vds_kdump_status_sp.sql:

Line 1: ----------------------------------------------------------------
Line 2: -- [vds_kdump_status] Table
Line 3: ----------------------------------------------------------------
Line 4: 
Line 5: -- UpsertKdumpStatus is used in fence_kdump listener
Again , what is the reason to introduce upsterts
As far as PG does not support that in its syntax, I see no reason not to have 
insert/update as any other entity
Line 6: CREATE OR REPLACE FUNCTION UpsertKdumpStatus(
Line 7:         v_vds_id UUID,
Line 8:         v_status VARCHAR(20),
Line 9:         v_address VARCHAR(50))


Line 5: -- UpsertKdumpStatus is used in fence_kdump listener
Line 6: CREATE OR REPLACE FUNCTION UpsertKdumpStatus(
Line 7:         v_vds_id UUID,
Line 8:         v_status VARCHAR(20),
Line 9:         v_address VARCHAR(50))
address should be 255
Line 10: RETURNS INT
Line 11:     AS $procedure$
Line 12: BEGIN
Line 13:     UPDATE vds_kdump_status


Line 23:                 address
Line 24:             ) VALUES (
Line 25:                 v_vds_id,
Line 26:                 v_status,
Line 27:                 v_address
address should be 255
Line 28:             );
Line 29:     END IF;
Line 30: 
Line 31:     RETURN 1;


Line 31:     RETURN 1;
Line 32: END; $procedure$
Line 33: LANGUAGE plpgsql;
Line 34: 
Line 35: CREATE OR REPLACE FUNCTION UpsertKdumpStatusForIp(
Same
Line 36:         v_ip VARCHAR(20),
Line 37:         v_status VARCHAR(20),
Line 38:         v_address VARCHAR(50))
Line 39: RETURNS INT


Line 34: 
Line 35: CREATE OR REPLACE FUNCTION UpsertKdumpStatusForIp(
Line 36:         v_ip VARCHAR(20),
Line 37:         v_status VARCHAR(20),
Line 38:         v_address VARCHAR(50))
address should be 255
Line 39: RETURNS INT
Line 40:     AS $procedure$
Line 41: DECLARE
Line 42:     v_vds_id UUID;


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I58696ddefa8602b1daa5f55ab4dc11bdc565e265
Gerrit-PatchSet: 12
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

Reply via email to