Eli Mesika has posted comments on this change. Change subject: core: user level queries - filtered entities ......................................................................
Patch Set 7: The following are answers to Allon comments Allon wrote: >>if the field list is static, why do we need dynamic SQL? Keep in mind that except of the 1st time the dynamic SQL is simply executing a cached sql for the specified object taken from object_object_column_white_list_sql table. >> Why not create another view, "user_vds"? Because duplicating the views just because you have to filter some columns is bad and hard to maintain. >>it will be a pain to maintain, but I don't see how adding >>another field to >>=such a view is conceptually different >>from adding another column to the >>whitelist? Adding a column to the white list is straight forward, you maintain security in one place. >> Moreover - if we forget to add a column, tests will >>break, and we'll know >> we forgot something. Forgetting to >>add a column to the white list will >> cause the function to >>return null, which will be much harder to catch. No, tests will not break since your tests are using the admin user you will not notice that you missed the value until you will found it on QE (if we are lucky) or in the customer site (if we are not). The worst that can be in my suggestion is that the added field value will be empty for users that are not belong to the Admin role and this is relatively easy to fix. -- To view, visit http://gerrit.ovirt.org/4469 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: If68b290aaacc0eea5d117ef64536cbf94d195cee Gerrit-PatchSet: 7 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Allon Mureinik <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Itamar Heim <[email protected]> Gerrit-Reviewer: Yair Zaslavsky <[email protected]> _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
