Mike Kolesnik has posted comments on this change.
Change subject: engine: Add label column to network
......................................................................
Patch Set 2:
(5 comments)
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/NetworkDao.java
Line 109: * @param id
Line 110: * the data-center id
Line 111: * @return all labels defined for the data-center's networks
Line 112: */
Line 113: List<String> getAllNetworkLabelsForDataCenter(Guid id);
I would expect the return value to be a Set
....................................................
File
backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/network/NetworkDaoDbFacadeImpl.java
Line 188: return new Network();
Line 189: }
Line 190: }
Line 191:
Line 192: private static class StringRowMapper implements RowMapper<String>
{
Please try to use org.springframework.jdbc.core.SingleColumnRowMapper<T> instead
Line 193: public static final StringRowMapper instance = new
StringRowMapper();
Line 194:
Line 195: @Override
Line 196: public String mapRow(ResultSet rs, int rowNum) throws
SQLException {
....................................................
File packaging/dbscripts/network_sp.sql
Line 259: LANGUAGE plpgsql;
Line 260:
Line 261:
Line 262: Create or replace FUNCTION GetAllNetworkLabelsByDataCenterId(v_id
UUID)
Line 263: RETURNS SETOF TEXT
You should mark this as STABLE
Line 264: AS $procedure$
Line 265: BEGIN
Line 266: RETURN QUERY
Line 267: SELECT label as label
Line 263: RETURNS SETOF TEXT
Line 264: AS $procedure$
Line 265: BEGIN
Line 266: RETURN QUERY
Line 267: SELECT label as label
Shouldn't you select distinct labels?
Line 268: FROM network
Line 269: WHERE network.storage_pool_id = v_id;
Line 270: END; $procedure$
Line 271: LANGUAGE plpgsql;
....................................................
File packaging/dbscripts/upgrade/03_04_0300_add_label_column_to_networks.sql
Line 1: select fn_db_add_column('network', 'label', 'text');
Why type text if this is only a short text field?
Would you expect a label with tens of thousands of characters?
--
To view, visit http://gerrit.ovirt.org/22634
To unsubscribe, visit http://gerrit.ovirt.org/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I4e3c309cef92e63ebe6f1f48f19f8848e9bceb70
Gerrit-PatchSet: 2
Gerrit-Project: ovirt-engine
Gerrit-Branch: master
Gerrit-Owner: Moti Asayag <masa...@redhat.com>
Gerrit-Reviewer: Mike Kolesnik <mkole...@redhat.com>
Gerrit-Reviewer: Moti Asayag <masa...@redhat.com>
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
