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

Reply via email to