Improve SQL generator by grouping "exists" statements better

packages/serveradmin/serveradmin/serverdb/sql_generator.py

@@ -271,56 +271,49 @@ def _real_condition_sql(attribute, template, related_vias):
     related_vias = related_vias[attribute.attribute_id]
     assert related_vias
 
-    # We start with the condition for the attributes the server has on
-    # its own.  Then, add the conditions for all possible relations.  They
-    # are going to be OR'ed together.
-    relation_conditions = []
+    attribute_conditions = (
+        "sub.attribute_id = '{0}'".format(attribute.attribute_id),
+        template.format('sub.value'),
+    )
+
+    # One condition per relation path.  The directly attached path stays a
+    # correlated EXISTS: with a single equality to "server" Postgres turns
+    # it into a hash semi join, or an anti join under NOT.  The inherited
+    # paths are deliberately *uncorrelated* subqueries instead - see
+    # _inherited_server_ids_sql() for why.
+    path_conditions = []
     for related_via_attribute, servertype_ids in related_vias.items():
         if related_via_attribute is None:
-            # The condition for directly attached attributes
-            relation_condition = 'server.server_id = sub.server_id'
-        elif related_via_attribute.type == 'supernet':
-            relation_condition = _supernet_exists_sql(
-                related_via_attribute, 'supernet',
-                '>>= server_addr.value',
-                (
-                    _target_servertype_sql('supernet', related_via_attribute),
-                    'supernet.server_id = sub.server_id'
-                ),
+            condition = _exists_sql(
+                model, 'sub',
+                ('server.server_id = sub.server_id',) + attribute_conditions,
             )
-        elif related_via_attribute.type == 'reverse':
-            relation_condition = _exists_sql(ServerRelationAttribute, 'rel1', (
-                "rel1.attribute_id = '{0}'".format(
-                    related_via_attribute.reversed_attribute_id
-                ),
-                'rel1.value = server.server_id',
-                'rel1.server_id = sub.server_id',
-            ))
         else:
-            assert related_via_attribute.type == 'relation'
-            relation_condition = _exists_sql(ServerRelationAttribute, 'rel1', (
-                "rel1.attribute_id = '{0}'"
-                .format(related_via_attribute.attribute_id),
-                'rel1.server_id = server.server_id',
-                'rel1.value = sub.server_id',
-            ))
-        relation_conditions.append((relation_condition, servertype_ids))
-
-    if len(relation_conditions) == 1:
-        mixed_relation_condition = relation_conditions[0][0]
-    else:
-        mixed_relation_condition = '({0})'.format(' OR '.join(
-            '({0} AND server.servertype_id IN ({1}))'
-            .format(relation_condition, ', '.join(
-                "'{0}'".format(s) for s in servertype_ids)
+            condition = '(server.server_id IN ({0}))'.format(
+                _inherited_server_ids_sql(
+                    model, attribute_conditions, related_via_attribute
+                )
             )
-            for relation_condition, servertype_ids in relation_conditions
-        ))
-
-    return _exists_sql(model, 'sub', (
-        mixed_relation_condition,
-        "sub.attribute_id = '{0}'".format(attribute.attribute_id),
-        template.format('sub.value'),
+        path_conditions.append((condition, servertype_ids))
+
+    if len(path_conditions) == 1:
+        return path_conditions[0][0]
+
+    # The paths are OR'ed together outside their subqueries, rather than
+    # inside a single EXISTS: Postgres cannot make a semi join out of an
+    # EXISTS whose correlation to "server" is an OR of alternatives, and
+    # falls back to a nested loop over every (server, sub) pair.
+    #
+    # The servertype guard comes first in each branch on purpose.  Postgres
+    # reorders AND clauses by cost at the top level only, not inside the
+    # branches of an OR, and otherwise evaluates left to right; the cheap
+    # test first lets it skip the subquery probe for servertypes that do
+    # not use that path at all.
+    return '({0})'.format(' OR '.join(
+        '(server.servertype_id IN ({0}) AND {1})'.format(
+            ', '.join("'{0}'".format(s) for s in servertype_ids), condition
+        )
+        for condition, servertype_ids in path_conditions
     ))
 
 
@@ -330,19 +323,100 @@ def _exists_sql(model, alias, conditions):
     )
 
 
-def _supernet_exists_sql(attribute: Attribute, supernet_alias: str, addr_match: str, where: tuple[str, ...]):
-    if attribute.inet_address_family:
-        af_join = (
-            (Attribute._meta.db_table, 'server_attr', ('server_attr.attribute_id = server_addr.attribute_id',)),
-            (Attribute._meta.db_table, 'net_attr', ('net_attr.attribute_id = net_addr.attribute_id',)),
-        )
-        af_where = (
-            f"net_attr.inet_address_family = '{attribute.inet_address_family}'",
-            f"server_attr.inet_address_family = '{attribute.inet_address_family}'",
-        )
+def _inherited_server_ids_sql(
+    model, attribute_conditions, related_via_attribute
+):
+    """SELECT the ids of servers inheriting a matching value via an attribute
+
+    Nothing in the returned subquery references the outer "server".  That is
+    the point: as a correlated EXISTS, an inherited path was re-evaluated
+    once per candidate server, re-finding the same matching rows every time
+    and then testing each (server, match) pair one by one - for a supernet
+    path that meant millions of inet containment checks, none of them able
+    to use the GiST index on server_inet_attribute.value because the plan
+    probed the server's address by key and only then filtered on
+    containment.  Uncorrelated, Postgres evaluates it once, hashes the ids,
+    and probes the hash per row; the containment join is driven from the
+    matching networks' prefixes, which is what the index is for.  The cost
+    becomes (candidates + matches) instead of their product.
+
+    The selected column is a NOT NULL foreign key in every branch, so the
+    caller's "server.server_id IN (...)" keeps plain set semantics under NOT
+    as well: no NULL can leak into the result and turn the test unknown.
+    """
+    sub_table = model._meta.db_table
+    rel_table = ServerRelationAttribute._meta.db_table
+    conditions = list(attribute_conditions)
+
+    if related_via_attribute.type == 'relation':
+        # The server points at the owner of the value.
+        select = 'rel1.server_id'
+        from_ = '{0} AS rel1'.format(rel_table)
+        joins = [
+            'JOIN {0} AS sub ON (sub.server_id = rel1.value)'.format(sub_table)
+        ]
+        conditions.insert(0, "rel1.attribute_id = '{0}'".format(
+            related_via_attribute.attribute_id
+        ))
+    elif related_via_attribute.type == 'reverse':
+        # The owner of the value points at the server.
+        select = 'rel1.value'
+        from_ = '{0} AS rel1'.format(rel_table)
+        joins = [
+            'JOIN {0} AS sub ON (sub.server_id = rel1.server_id)'
+            .format(sub_table)
+        ]
+        conditions.insert(0, "rel1.attribute_id = '{0}'".format(
+            related_via_attribute.reversed_attribute_id
+        ))
     else:
-        af_join = ()
-        af_where = ()
+        assert related_via_attribute.type == 'supernet'
+        # The owner of the value is a network containing one of the
+        # server's addresses on the same inet attribute.  Same join graph
+        # as _supernet_exists_sql(), minus the correlation to "server".
+        af_join, af_where = _supernet_af_sql(related_via_attribute)
+        select = 'server_addr.server_id'
+        from_ = '{0} AS sub'.format(sub_table)
+        joins = [
+            'JOIN {0} AS supernet ON (supernet.server_id = sub.server_id)'
+            .format(Server._meta.db_table),
+            'JOIN {0} AS net_addr ON (net_addr.server_id = supernet.server_id)'
+            .format(ServerInetAttribute._meta.db_table),
+            'JOIN {0} AS server_addr ON ('
+            'server_addr.attribute_id = net_addr.attribute_id'
+            ' AND net_addr.value >>= server_addr.value)'
+            .format(ServerInetAttribute._meta.db_table),
+        ] + [
+            f'JOIN {x[0]} AS {x[1]} ON ({" AND ".join(x[2])})' for x in af_join
+        ]
+        conditions.insert(
+            0, _target_servertype_sql('supernet', related_via_attribute)
+        )
+        conditions.extend(af_where)
+
+    return 'SELECT {0} FROM {1} {2} WHERE {3}'.format(
+        select, from_, ' '.join(joins),
+        ' AND '.join(c for c in conditions if c),
+    )
+
+
+def _supernet_af_sql(attribute):
+    """Joins and conditions pinning a supernet match to one address family"""
+    if not attribute.inet_address_family:
+        return (), ()
+    af_join = (
+        (Attribute._meta.db_table, 'server_attr', ('server_attr.attribute_id = server_addr.attribute_id',)),
+        (Attribute._meta.db_table, 'net_attr', ('net_attr.attribute_id = net_addr.attribute_id',)),
+    )
+    af_where = (
+        f"net_attr.inet_address_family = '{attribute.inet_address_family}'",
+        f"server_attr.inet_address_family = '{attribute.inet_address_family}'",
+    )
+    return af_join, af_where
+
+
+def _supernet_exists_sql(attribute: Attribute, supernet_alias: str, addr_match: str, where: tuple[str, ...]):
+    af_join, af_where = _supernet_af_sql(attribute)
 
     joins = (
         (ServerInetAttribute._meta.db_table, 'server_addr', ('server_addr.server_id = server.server_id',)),

packages/serveradmin/serveradmin/serverdb/tests/test_inherited_filters.py

@@ -0,0 +1,67 @@
+"""Serveradmin - Filters on inherited attributes
+
+Copyright (c) 2026 InnoGames GmbH
+"""
+
+from django.contrib.auth.models import User
+from django.test import TransactionTestCase
+
+from adminapi.filters import Not
+from serveradmin.dataset import Query
+from serveradmin.serverdb.models import ServertypeAttribute
+
+
+class InheritedFilterTest(TransactionTestCase):
+    """Filter on an attribute that some servertypes inherit
+
+    The fixture has vm-1 pointing at hv-1 through the "hypervisor" relation
+    attribute, with "vms" as its reverse, and neither servertype carrying
+    "os".  Each test wires "os" onto one of them directly and onto the other
+    through a relation path, then filters on it without narrowing the
+    servertype, so the SQL generator has to render both the direct and the
+    inherited path (see sql_generator._inherited_server_ids_sql).  The
+    supernet path is covered in test_ip_addr_type.
+    """
+
+    fixtures = ['auth_user.json', 'test_dataset.json']
+
+    def _set_os(self, hostname, value):
+        query = Query({'hostname': hostname}, ['os'])
+        query.update(os=value)
+        query.commit(user=User.objects.first())
+
+    def _hostnames(self, os_filter):
+        return {s['hostname'] for s in Query({'os': os_filter}, ['hostname'])}
+
+    def test_inherited_via_relation(self):
+        # hypervisor carries "os", vm inherits it from its hypervisor.
+        ServertypeAttribute.objects.create(
+            servertype_id='hypervisor', attribute_id='os',
+        )
+        ServertypeAttribute.objects.create(
+            servertype_id='vm', attribute_id='os',
+            related_via_attribute_id='hypervisor',
+        )
+        self._set_os('hv-1', 'buster')
+
+        self.assertEqual(self._hostnames('buster'), {'hv-1', 'vm-1'})
+
+        # The inheriting server must drop out under negation too: the
+        # subquery selects a NOT NULL column, so NOT IN keeps set semantics.
+        excluded = self._hostnames(Not('buster'))
+        self.assertNotIn('hv-1', excluded)
+        self.assertNotIn('vm-1', excluded)
+        self.assertIn('test0', excluded)
+
+    def test_inherited_via_reverse(self):
+        # vm carries "os", hypervisor inherits it from its vms.
+        ServertypeAttribute.objects.create(
+            servertype_id='vm', attribute_id='os',
+        )
+        ServertypeAttribute.objects.create(
+            servertype_id='hypervisor', attribute_id='os',
+            related_via_attribute_id='vms',
+        )
+        self._set_os('vm-1', 'buster')
+
+        self.assertEqual(self._hostnames('buster'), {'vm-1', 'hv-1'})

packages/serveradmin/serveradmin/serverdb/tests/test_ip_addr_type.py

@@ -788,6 +788,33 @@ def test_af_unaware_supernet_related(self):
             (self.rn_ipv4["hostname"], self.rn_ipv6["hostname"]),
         )
 
+    def test_related_attribute_without_servertype_filter(self):
+        # Filtering on an attribute that some servertypes carry directly and
+        # others inherit, without narrowing the servertype, makes the SQL
+        # generator emit one EXISTS per relation path (see
+        # sql_generator._real_condition_sql).  Both paths have to deliver:
+        # the route networks match directly, server_rn through its
+        # AF-unaware supernet.  server_pn sits in no route network.
+        hostnames = {
+            server["hostname"]
+            for server in Query(
+                {
+                    "network_type": filters.Any(
+                        "internal_ipv4", "internal_ipv6"
+                    ),
+                },
+                ["hostname"],
+            )
+        }
+        self.assertEqual(
+            hostnames,
+            {
+                self.rn_ipv4["hostname"],
+                self.rn_ipv6["hostname"],
+                self.server_rn["hostname"],
+            },
+        )
+
     def test_af_aware_supernet(self):
         # Querying for AF-aware supernet attribute will find only objects
         # matching the given address family.