Eli Mesika has posted comments on this change. Change subject: core: WIP: Adding JPA to ovirt ......................................................................
Patch Set 16: Code-Review-1 (1 comment) http://gerrit.ovirt.org/#/c/22806/16/backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java File backend/manager/modules/dal/src/main/java/org/ovirt/engine/core/dao/VdsGroupDAODbFacadeImpl.java: Line 40: Query query; Line 41: Line 42: if (isFiltered) { Line 43: query = Line 44: em.createQuery("select distinct g from VDSGroup g inner join g.userIds u where u.id.userId = :userId and g.id = :id"); General comment to all those static select strings This is a maintenance hell when we come to schema changes that should be applied all over those queries All those queries should be accessed from one place (singelton with a static method that gets the key and give the SQL) The key should be the a composed key of the Business entity and the name of the SP we had used instead of the SQL for easy access and compare with past versions (keep in mind that today we have a file per entity) Also, SQL changes resulted from adding a column to an entity will be in the same class I must say that I am not for hybrid solutions, they proved themselves as time wasters in the past and I think that if we are not going for full JPA end-to-end this is a waste of time Anyway, giving -1 until all SQL strings are encapsulated in one place. Line 45: query.setParameter("userId", userID); Line 46: } else { Line 47: query = em.createQuery("select distinct g from VDSGroup g where g.id = :id"); Line 48: } -- To view, visit http://gerrit.ovirt.org/22806 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I3cd0bbf9f0913955cb3e1facfa9a4bdc1f1ab24d Gerrit-PatchSet: 16 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Liran Zelkha <[email protected]> Gerrit-Reviewer: Arik Hadas <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Gustavo Frederico Temple Pedrosa <[email protected]> Gerrit-Reviewer: Itamar Heim <[email protected]> Gerrit-Reviewer: Liran Zelkha <[email protected]> Gerrit-Reviewer: Mike Kolesnik <[email protected]> Gerrit-Reviewer: Omer Frenkel <[email protected]> Gerrit-Reviewer: Tal Nisan <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
