Frank Kobzik has posted comments on this change. Change subject: core: Fix splitting logic for ip addresses ......................................................................
Patch Set 2: I don't think that's straightforward solution, for example: - we want to get list of sorted vms by ip address, so we must add fn_get_comparable_ip_list(vm_ip) to the select clause and the query executes ok (the query will look like this: 'select vms.*, fn_blah(vm_ip) from vms ... order by fn_blah(vm_ip)). - now imagine a similar situation in which we want to sort by vm name, engine would construct query like this one 'select vms.*,vm_name from vms ... order by vm_name'. in this case postgres will complain about ambiguity of vm_name column in select clause. my point is that the logic that would insert sorting column name in the select clause would have to have knowledge about the table on which the select is executed, to prevent ambiguous columns as in the 2nd example. Another solution would be to add special case to SyntaxChecker to handle this specific situation (if there is fn_get_sorted_ip(vm_ip), then add this string to select as well). But that's not nice at all. I still think the solution with extra added column is the best one. -- To view, visit http://gerrit.ovirt.org/36946 To unsubscribe, visit http://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I6923e1d99185e9aca69036edda00293ed1718b92 Gerrit-PatchSet: 2 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Eli Mesika <emes...@redhat.com> Gerrit-Reviewer: Frank Kobzik <fkob...@redhat.com> Gerrit-Reviewer: Lior Vernia <lver...@redhat.com> Gerrit-Reviewer: automat...@ovirt.org Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: No _______________________________________________ Engine-patches mailing list Engine-patches@ovirt.org http://lists.ovirt.org/mailman/listinfo/engine-patches