Fixes #17362: Fix unicity of VRF returned by filter_present_in_vrf function

[pl0xym0r]

Fixes: #17362

.distinct() permits to remove unnecessary duplicated returns of the VRF passed in parameter in the function.
This permits to have unique IP and Prefixes when using present_in_vrf / present_in_vrf_id parameter, example :

For prefixes:

For IP:

[jeremystretch]

Thanks for this @pl0xym0r. Did you happen to try any alternative approaches to adjusting the queryset, specifically filtering for the VRF IDs separately and then passing them as a list for the base queryset. I'm curious what, if any, difference in performance that would have versus calling distinct().

[pl0xym0r]

TBH, it seemed the easiest way to solve the issue and I didn't try to modify the code itself, nor took in mind any performance issue as .distinct() is used multiple times on DCIM filtersets also.
The more import/export RT you implement on a VRF, the more duplicated results are returned for this VRF.
Should I try to remove the bitwise and use queryset1.union(queryset2) instead ?

[bctiemann]

FWIW addding distinct() looks like it adds some cost to the query, but maybe not a dealbreaker. Here are comparisons of Prefix and IPAddress without and with DISTINCT:

Prefix:

                                           QUERY PLAN                                            
-------------------------------------------------------------------------------------------------
 Sort  (cost=11.53..11.75 rows=86 width=97)
   Sort Key: ipam_prefix.vrf_id NULLS FIRST, ipam_prefix.prefix, ipam_prefix.id
   ->  Hash Join  (cost=4.83..8.77 rows=86 width=97)
         Hash Cond: (ipam_prefix.vrf_id = ipam_vrf.id)
         Join Filter: ((ipam_prefix.vrf_id = 1) OR (hashed SubPlan 1))
         ->  Seq Scan on ipam_prefix  (cost=0.00..2.90 rows=90 width=97)
         ->  Hash  (cost=2.29..2.29 rows=11 width=16)
               ->  Hash Right Join  (cost=1.14..2.29 rows=11 width=16)
                     Hash Cond: (ipam_vrf_export_targets.vrf_id = ipam_vrf.id)
                     ->  Seq Scan on ipam_vrf_export_targets  (cost=0.00..1.11 rows=11 width=16)
                     ->  Hash  (cost=1.06..1.06 rows=6 width=8)
                           ->  Seq Scan on ipam_vrf  (cost=0.00..1.06 rows=6 width=8)
         SubPlan 1
           ->  Nested Loop  (cost=0.00..2.41 rows=1 width=8)
                 Join Filter: (u0.id = u1.routetarget_id)
                 ->  Seq Scan on ipam_vrf_import_targets u1  (cost=0.00..1.14 rows=1 width=8)
                       Filter: (vrf_id = 1)
                 ->  Seq Scan on ipam_routetarget u0  (cost=0.00..1.12 rows=12 width=8)
(18 rows)

Prefix with DISTINCT:

 Unique  (cost=11.53..15.40 rows=86 width=97)
   ->  Sort  (cost=11.53..11.75 rows=86 width=97)
         Sort Key: ipam_prefix.vrf_id NULLS FIRST, ipam_prefix.prefix, ipam_prefix.id, ipam_prefix.created, ipam_prefix.last_updated, ipam_prefix.custom_field_data, ipam_prefix.description, ipam_prefix.comments, ipam_prefix.site_id, ipam_prefix.tenant_id, ipam_prefix.vlan_id, ipam_prefix.status, ipam_prefix.role_id, ipam_prefix.is_pool, ipam_prefix.mark_utilized, ipam_prefix._depth, ipam_prefix._children
         ->  Hash Join  (cost=4.83..8.77 rows=86 width=97)
               Hash Cond: (ipam_prefix.vrf_id = ipam_vrf.id)
               Join Filter: ((ipam_prefix.vrf_id = 1) OR (hashed SubPlan 1))
               ->  Seq Scan on ipam_prefix  (cost=0.00..2.90 rows=90 width=97)
               ->  Hash  (cost=2.29..2.29 rows=11 width=16)
                     ->  Hash Right Join  (cost=1.14..2.29 rows=11 width=16)
                           Hash Cond: (ipam_vrf_export_targets.vrf_id = ipam_vrf.id)
                           ->  Seq Scan on ipam_vrf_export_targets  (cost=0.00..1.11 rows=11 width=16)
                           ->  Hash  (cost=1.06..1.06 rows=6 width=8)
                                 ->  Seq Scan on ipam_vrf  (cost=0.00..1.06 rows=6 width=8)
               SubPlan 1
                 ->  Nested Loop  (cost=0.00..2.41 rows=1 width=8)
                       Join Filter: (u0.id = u1.routetarget_id)
                       ->  Seq Scan on ipam_vrf_import_targets u1  (cost=0.00..1.14 rows=1 width=8)
                             Filter: (vrf_id = 1)
                       ->  Seq Scan on ipam_routetarget u0  (cost=0.00..1.12 rows=12 width=8)
(19 rows)

IPAddress:

                                          QUERY PLAN                                          
----------------------------------------------------------------------------------------------
 Sort  (cost=22.49..22.97 rows=193 width=115)
   Sort Key: ((host(ipam_ipaddress.address))::inet)
   ->  Hash Left Join  (cost=4.79..15.17 rows=193 width=115)
         Hash Cond: (ipam_vrf.id = ipam_vrf_export_targets.vrf_id)
         Filter: ((ipam_ipaddress.vrf_id = 1) OR (hashed SubPlan 1))
         ->  Hash Join  (cost=1.14..6.69 rows=180 width=91)
               Hash Cond: (ipam_ipaddress.vrf_id = ipam_vrf.id)
               ->  Seq Scan on ipam_ipaddress  (cost=0.00..4.80 rows=180 width=83)
               ->  Hash  (cost=1.06..1.06 rows=6 width=8)
                     ->  Seq Scan on ipam_vrf  (cost=0.00..1.06 rows=6 width=8)
         ->  Hash  (cost=1.11..1.11 rows=11 width=16)
               ->  Seq Scan on ipam_vrf_export_targets  (cost=0.00..1.11 rows=11 width=16)
         SubPlan 1
           ->  Nested Loop  (cost=0.00..2.41 rows=1 width=8)
                 Join Filter: (u0.id = u1.routetarget_id)
                 ->  Seq Scan on ipam_vrf_import_targets u1  (cost=0.00..1.14 rows=1 width=8)
                       Filter: (vrf_id = 1)
                 ->  Seq Scan on ipam_routetarget u0  (cost=0.00..1.12 rows=12 width=8)
(18 rows)

IPAddress with DISTINCT:

 Unique  (cost=22.49..30.69 rows=180 width=115)
   ->  Sort  (cost=22.49..22.97 rows=193 width=115)
         Sort Key: ((host(ipam_ipaddress.address))::inet), ipam_ipaddress.id, ipam_ipaddress.created, ipam_ipaddress.last_updated, ipam_ipaddress.custom_field_data, ipam_ipaddress.description, ipam_ipaddress.comments, ipam_ipaddress.address, ipam_ipaddress.vrf_id, ipam_ipaddress.tenant_id, ipam_ipaddress.status, ipam_ipaddress.role, ipam_ipaddress.assigned_object_type_id, ipam_ipaddress.assigned_object_id, ipam_ipaddress.nat_inside_id, ipam_ipaddress.dns_name
         ->  Hash Left Join  (cost=4.79..15.17 rows=193 width=115)
               Hash Cond: (ipam_vrf.id = ipam_vrf_export_targets.vrf_id)
               Filter: ((ipam_ipaddress.vrf_id = 1) OR (hashed SubPlan 1))
               ->  Hash Join  (cost=1.14..6.69 rows=180 width=91)
                     Hash Cond: (ipam_ipaddress.vrf_id = ipam_vrf.id)
                     ->  Seq Scan on ipam_ipaddress  (cost=0.00..4.80 rows=180 width=83)
                     ->  Hash  (cost=1.06..1.06 rows=6 width=8)
                           ->  Seq Scan on ipam_vrf  (cost=0.00..1.06 rows=6 width=8)
               ->  Hash  (cost=1.11..1.11 rows=11 width=16)
                     ->  Seq Scan on ipam_vrf_export_targets  (cost=0.00..1.11 rows=11 width=16)
               SubPlan 1
                 ->  Nested Loop  (cost=0.00..2.41 rows=1 width=8)
                       Join Filter: (u0.id = u1.routetarget_id)
                       ->  Seq Scan on ipam_vrf_import_targets u1  (cost=0.00..1.14 rows=1 width=8)
                             Filter: (vrf_id = 1)
                       ->  Seq Scan on ipam_routetarget u0  (cost=0.00..1.12 rows=12 width=8)
(19 rows)

That's about a 33% slowdown in both cases.

[jeremystretch]

Seems like a reasonable fix IMO. We can investigate potential optimizations if need be in the future but this should at least resolve the bug.