)]}'
{"neutron_lib/db/model_query.py":[{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e470ffe93601c8a0768f600fa4dcbfbef04cf0a1","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c04127ad_a4e6a742","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":50},"updated":"2021-05-07 14:06:04.000000000","message":"When this query makes sense? In other words: if we retrieve the whole model (\"port\", for example), we\u0027ll also retrieve all backrefs related to \"port\" (\"network\", \"standartattrs\", etc). In this case, why do we need to add an extra field that is not related to \"port\"?\n\nIn the other case, when we defined a \"field\" parameter, the query will be restricted to this single column. Does it make sense then?","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"07c1c8696692d4008c15458307cc92b267749763","unresolved":true,"context_lines":[{"line_number":149,"context_line":"    if query_filter is not None:"},{"line_number":150,"context_line":"        query \u003d query.filter(query_filter)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"569ea781_188f753a","line":154,"range":{"start_line":152,"start_character":4,"end_line":154,"end_character":50},"updated":"2021-05-25 17:10:11.000000000","message":"nit: \n\n    lazy_fields \u003d lazy_fields or []\n    ...\n    for field in lazy_fields:\n        query \u003d query.options(lazyload(field))","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"e07997c838a844d0c49b63a2d0b1644b062efab9","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"c5d510d3_db94e2aa","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":50},"in_reply_to":"513ea348_65f0dfcb","updated":"2021-05-12 07:03:45.000000000","message":"How are we going to override the default load mode? Now we can (1) load the default OVO, using the DB references or (2) load a single DB register column. IMO, to accept this patch we need a new loading strategy that will allow this kind of queries.","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"73871ef8b1abe1dfc4e8273b92203d3d637550d1","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9402a169_06fed53d","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":50},"in_reply_to":"513ea348_65f0dfcb","updated":"2021-05-13 04:39:01.000000000","message":"This change is not quite related to OVO and works at a lower when full model is queried (not a single field). We  can override the default load mode according to [1]: \"Lazy loading can be enabled for a given attribute that is normally configured in some other way using the lazyload() loader option\" - which is also confirmed with tests that I performed.\n\nFor example of usage please check https://review.opendev.org/c/openstack/neutron/+/790691\n\n[1] https://docs.sqlalchemy.org/en/14/orm/loading_relationships.html#lazy-loading","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2c8801be14a5b80a5ca2b5541e98d9ca944861cc","unresolved":true,"context_lines":[{"line_number":149,"context_line":"    if query_filter is not None:"},{"line_number":150,"context_line":"        query \u003d query.filter(query_filter)"},{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"429d458b_d6f2ed79","line":154,"range":{"start_line":152,"start_character":4,"end_line":154,"end_character":50},"in_reply_to":"569ea781_188f753a","updated":"2021-05-26 08:56:10.000000000","message":"looks nicer indeed, will update if have to re-spin, thanks","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":16688,"name":"Rodolfo Alonso","email":"ralonsoh@redhat.com","username":"rodolfo-alonso-hernandez"},"change_message_id":"07c1c8696692d4008c15458307cc92b267749763","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"ed914950_3b2dab18","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":50},"in_reply_to":"9402a169_06fed53d","updated":"2021-05-25 17:10:11.000000000","message":"Right, but the example you provided in [1] does nothing to the SQL query. The fields to added to be lazyloaded in [2] are not in the port query because those three child registers are loaded as subqueries [3][4][5].\n\nI tested with a parameter that is loaded using a join [6]: port.qos_policy_binding. I\u0027ve tested that the query removes this parameter from the output and from the SQL query:\n- With qos_policy_binding: http://paste.openstack.org/show/805708/\n- Lazyloading qos_policy_binding: http://paste.openstack.org/show/805709/\n\nSo yes, this patch makes sense to \"peel off\", from some specific SQL queries, some unneeded fields that are not going to be read in the retrieved DB object. That could reduce the complexity and thus the query time.\n\nOf course, this new feature must be use with care: those fields that are not in the main SQL query will be retrieved, when read from the DB resultant object, using secondary queries. That could increase the DB access time.\n\n[1]https://review.opendev.org/c/openstack/neutron/+/790691\n[2]https://review.opendev.org/c/openstack/neutron/+/790691/2/neutron/db/db_base_plugin_v2.py#1527\n[3]https://github.com/openstack/neutron/blob/dd24662e5316a72fc5b9cbfe36bec1705ce2c505/neutron/plugins/ml2/models.py#L92\n[4]https://github.com/openstack/neutron/blob/dd24662e5316a72fc5b9cbfe36bec1705ce2c505/neutron/plugins/ml2/models.py#L132\n[5]https://github.com/openstack/neutron/blob/dd24662e5316a72fc5b9cbfe36bec1705ce2c505/neutron/db/models/port_forwarding.py#L60\n[6]https://github.com/openstack/neutron/blob/dd24662e5316a72fc5b9cbfe36bec1705ce2c505/neutron/db/qos/models.py#L123","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"b1f0d28d65ababacf709ae093efb8eae82cd26a9","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"513ea348_65f0dfcb","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":50},"in_reply_to":"c04127ad_a4e6a742","updated":"2021-05-11 09:53:13.000000000","message":"\u003e When this query makes sense? In other words: if we retrieve the whole model (\"port\", for example), we\u0027ll also retrieve all backrefs related to \"port\" (\"network\", \"standartattrs\", etc). In this case, why do we need to add an extra field that is not related to \"port\"?\n\u003e \n\nThe idea is to override default load mode, so we don\u0027t retrieve all backrefs related to port, instead we set \u0027lazy\u0027 mode for those which we don\u0027t need in current case. I\u0027ll upload a patch to neutron soon.\n\n\u003e In the other case, when we defined a \"field\" parameter, the query will be restricted to this single column. Does it make sense then?\n\nNot quite, this case is when we need most of the fields.","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"},{"author":{"_account_id":5948,"name":"Oleg Bondarev","email":"obondarev@mirantis.com","username":"obondarev"},"change_message_id":"2c8801be14a5b80a5ca2b5541e98d9ca944861cc","unresolved":true,"context_lines":[{"line_number":151,"context_line":""},{"line_number":152,"context_line":"    if lazy_fields:"},{"line_number":153,"context_line":"        for field in lazy_fields:"},{"line_number":154,"context_line":"            query \u003d query.options(lazyload(field))"},{"line_number":155,"context_line":"    return query"},{"line_number":156,"context_line":""},{"line_number":157,"context_line":""}],"source_content_type":"text/x-python","patch_set":2,"id":"9dacd6ff_ad925317","line":154,"range":{"start_line":154,"start_character":20,"end_line":154,"end_character":50},"in_reply_to":"ed914950_3b2dab18","updated":"2021-05-26 08:56:10.000000000","message":"Thank for analysis Rodolfo! Indeed there must be perf testing done before any new lazy_fields are added to a query","commit_id":"a165d9608047ec97a270050bf2ce2978d368152c"}]}