SanderMertens#1974 Fix issue w/cascade and deleting empty tables

Author: SanderMertens

distr/flecs.c

@@ -78056,6 +78056,23 @@ void ecs_iter_skip(
  */
 
 
+/* Check if group is currently linked in the cache group list. */
+static
+bool flecs_query_cache_group_is_linked(
+    const ecs_query_cache_t *cache,
+    const ecs_query_cache_group_t *group)
+{
+    ecs_query_cache_group_t *cur = cache->first_group;
+    while (cur) {
+        if (cur == group) {
+            return true;
+        }
+        cur = cur->next;
+    }
+
+    return false;
+}
+
 /* Get group id for table. */
 static
 uint64_t flecs_query_cache_get_group_id(
@@ -78095,11 +78112,6 @@ void flecs_query_cache_group_insert(
             cache->cascade_by - 1].src.id & EcsDesc) != 0;
     }
 
-    if (!group->info.id) {
-        /* Default group is always inserted */
-        return;
-    }
-
     ecs_query_cache_group_t *cur = cache->first_group, *prev = NULL;
     do {
         ecs_assert(cur->info.id != group->info.id, ECS_INTERNAL_ERROR, NULL);
@@ -78139,11 +78151,15 @@ ecs_query_cache_group_t* flecs_query_cache_ensure_group(
                     &cache->groups, ecs_query_cache_group_t, 0);
                 *group_ptr = group;
             }
-        }
 
-        if (cache->on_group_create) {
-            group->info.ctx = cache->on_group_create(
-                cache->query->world, 0, cache->group_by_ctx);
+            if (!flecs_query_cache_group_is_linked(cache, group)) {
+                flecs_query_cache_group_insert(cache, group);
+            }
+
+            if (cache->on_group_create) {
+                group->info.ctx = cache->on_group_create(
+                    cache->query->world, 0, cache->group_by_ctx);
+            }
         }
 
         return group;
@@ -78193,6 +78209,9 @@ void flecs_query_cache_group_fini(
 
     ecs_allocator_t *a = &cache->query->real_world->allocator;
     ecs_vec_fini(a, &group->tables, elem_size);
+    group->info.table_count = 0;
+    group->info.match_count = 0;
+    group->info.ctx = NULL;
 
     if (group != &cache->default_group) {
         ecs_map_remove_free(&cache->groups, group->info.id);

src/query/cache/group.c

@@ -5,6 +5,23 @@
 
 #include "../../private_api.h"
 
+/* Check if group is currently linked in the cache group list. */
+static
+bool flecs_query_cache_group_is_linked(
+    const ecs_query_cache_t *cache,
+    const ecs_query_cache_group_t *group)
+{
+    ecs_query_cache_group_t *cur = cache->first_group;
+    while (cur) {
+        if (cur == group) {
+            return true;
+        }
+        cur = cur->next;
+    }
+
+    return false;
+}
+
 /* Get group id for table. */
 static
 uint64_t flecs_query_cache_get_group_id(
@@ -44,11 +61,6 @@ void flecs_query_cache_group_insert(
             cache->cascade_by - 1].src.id & EcsDesc) != 0;
     }
 
-    if (!group->info.id) {
-        /* Default group is always inserted */
-        return;
-    }
-
     ecs_query_cache_group_t *cur = cache->first_group, *prev = NULL;
     do {
         ecs_assert(cur->info.id != group->info.id, ECS_INTERNAL_ERROR, NULL);
@@ -88,11 +100,15 @@ ecs_query_cache_group_t* flecs_query_cache_ensure_group(
                     &cache->groups, ecs_query_cache_group_t, 0);
                 *group_ptr = group;
             }
-        }
 
-        if (cache->on_group_create) {
-            group->info.ctx = cache->on_group_create(
-                cache->query->world, 0, cache->group_by_ctx);
+            if (!flecs_query_cache_group_is_linked(cache, group)) {
+                flecs_query_cache_group_insert(cache, group);
+            }
+
+            if (cache->on_group_create) {
+                group->info.ctx = cache->on_group_create(
+                    cache->query->world, 0, cache->group_by_ctx);
+            }
         }
 
         return group;
@@ -142,6 +158,9 @@ void flecs_query_cache_group_fini(
 
     ecs_allocator_t *a = &cache->query->real_world->allocator;
     ecs_vec_fini(a, &group->tables, elem_size);
+    group->info.table_count = 0;
+    group->info.match_count = 0;
+    group->info.ctx = NULL;
 
     if (group != &cache->default_group) {
         ecs_map_remove_free(&cache->groups, group->info.id);

test/query/project.json

@@ -1818,6 +1818,7 @@
                 "2_self_up_terms_new_tables",
                 "this_self_up_childof_pair_new_tables",
                 "up_w_delete_table_and_move_parent",
+                "cascade_default_group_reinsert_after_empty_table_delete",
                 "it_ptrs",
                 "it_ptrs_after_column_resize",
                 "it_ptrs_after_column_merge",

test/query/src/Cached.c

@@ -3763,6 +3763,58 @@ void Cached_up_w_delete_table_and_move_parent(void) {
     ecs_fini(world);
 }
 
+void Cached_cascade_default_group_reinsert_after_empty_table_delete(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+
+    ecs_query_t *q = ecs_query(world, {
+        .expr = "Position, ?Position(cascade)",
+        .cache_kind = EcsQueryCacheAuto
+    });
+    test_assert(q != NULL);
+
+    ecs_entity_t p = ecs_new(world);
+    ecs_entity_t c = ecs_new_w_pair(world, EcsChildOf, p);
+    ecs_set(world, c, Position, {10, 20});
+
+    ecs_entity_t e1 = ecs_new(world);
+    ecs_set(world, e1, Position, {1, 2});
+
+    ecs_iter_t it = ecs_query_iter(world, q);
+    test_bool(true, ecs_query_next(&it));
+    test_bool(true, ecs_query_next(&it));
+    test_bool(false, ecs_query_next(&it));
+
+    ecs_delete(world, e1);
+
+    int32_t deleted = ecs_delete_empty_tables(world, &(ecs_delete_empty_tables_desc_t) {
+        .delete_generation = 1
+    });
+    deleted += ecs_delete_empty_tables(world, &(ecs_delete_empty_tables_desc_t) {
+        .delete_generation = 1
+    });
+    test_assert(deleted > 0);
+
+    ecs_entity_t e2 = ecs_new(world);
+    ecs_set(world, e2, Position, {3, 4});
+
+    ecs_delete(world, e2);
+
+    /* Reproduces assert in flecs_query_cache_remove_group:
+     * ecs_assert(cur != NULL, ECS_INTERNAL_ERROR, NULL);
+     */
+    deleted = ecs_delete_empty_tables(world, &(ecs_delete_empty_tables_desc_t) {
+        .delete_generation = 1
+    });
+    deleted += ecs_delete_empty_tables(world, &(ecs_delete_empty_tables_desc_t) {
+        .delete_generation = 1
+    });
+    test_assert(deleted > 0);
+
+    ecs_fini(world);
+}
+
 void Cached_it_ptrs(void) {
     ecs_world_t *world = ecs_mini();
 

test/query/src/main.c

@@ -1754,6 +1754,7 @@ void Cached_rematch_empty_table_w_superset(void);
 void Cached_2_self_up_terms_new_tables(void);
 void Cached_this_self_up_childof_pair_new_tables(void);
 void Cached_up_w_delete_table_and_move_parent(void);
+void Cached_cascade_default_group_reinsert_after_empty_table_delete(void);
 void Cached_it_ptrs(void);
 void Cached_it_ptrs_after_column_resize(void);
 void Cached_it_ptrs_after_column_merge(void);
@@ -9656,6 +9657,10 @@ bake_test_case Cached_testcases[] = {
         "up_w_delete_table_and_move_parent",
         Cached_up_w_delete_table_and_move_parent
     },
+    {
+        "cascade_default_group_reinsert_after_empty_table_delete",
+        Cached_cascade_default_group_reinsert_after_empty_table_delete
+    },
     {
         "it_ptrs",
         Cached_it_ptrs
@@ -13896,7 +13901,7 @@ static bake_test_suite suites[] = {
         "Cached",
         NULL,
         NULL,
-        161,
+        162,
         Cached_testcases
     },
     {