Fix crash when sorted query with MatchEmptyTables flag only matches e…

…mpty tables Differential Revision: D66028916 Pull Request resolved: #1441
SanderMertens · Nov 15, 2024 · 45b242e · 45b242e
1 parent 544cccc
commit 45b242e
Show file tree

Hide file tree

Showing 5 changed files with 89 additions and 2 deletions.
diff --git a/distr/flecs.c b/distr/flecs.c
@@ -68023,6 +68023,12 @@ ecs_query_cache_t* flecs_query_cache_init(
     ecs_flags32_t query_flags = const_desc->flags | world->default_query_flags;
     desc.flags |= EcsQueryMatchEmptyTables | EcsQueryTableOnly | EcsQueryNested;
 
+    /* order_by is not compatible with matching empty tables, as it causes
+     * a query to return table slices, not entire tables. */
+    if (const_desc->order_by_callback) {
+        query_flags &= ~EcsQueryMatchEmptyTables;
+    }
+
     ecs_query_t *q = result->query = ecs_query_init(world, &desc);
     if (!q) {
         goto error;

diff --git a/src/query/engine/cache.c b/src/query/engine/cache.c
@@ -1238,6 +1238,12 @@ ecs_query_cache_t* flecs_query_cache_init(
     ecs_flags32_t query_flags = const_desc->flags | world->default_query_flags;
     desc.flags |= EcsQueryMatchEmptyTables | EcsQueryTableOnly | EcsQueryNested;
 
+    /* order_by is not compatible with matching empty tables, as it causes
+     * a query to return table slices, not entire tables. */
+    if (const_desc->order_by_callback) {
+        query_flags &= ~EcsQueryMatchEmptyTables;
+    }
+
     ecs_query_t *q = result->query = ecs_query_init(world, &desc);
     if (!q) {
         goto error;

diff --git a/test/query/project.json b/test/query/project.json
@@ -2120,7 +2120,9 @@
                 "sort_by_wildcard",
                 "sort_not_term",
                 "sort_or_term",
-                "sort_optional_term"
+                "sort_optional_term",
+                "order_empty_table",
+                "order_empty_table_only"
             ]
         }, {
             "id": "OrderByEntireTable",

diff --git a/test/query/src/OrderBy.c b/test/query/src/OrderBy.c
@@ -2172,3 +2172,66 @@ void OrderBy_sort_w_nontrivial_component(void) {
 
     ecs_fini(world);
 }
+
+void OrderBy_order_empty_table(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+    ECS_TAG(world, Foo);
+
+    ecs_entity_t e1 = ecs_insert(world, ecs_value(Position, {3, 0}));
+    ecs_entity_t e2 = ecs_insert(world, ecs_value(Position, {1, 0}));
+    ecs_entity_t e3 = ecs_insert(world, ecs_value(Position, {5, 0}));
+    ecs_entity_t e4 = ecs_insert(world, ecs_value(Position, {2, 0}));
+    ecs_entity_t e5 = ecs_insert(world, ecs_value(Position, {4, 0}));
+
+    // Create empty table
+    ecs_add(world, e5, Foo);
+    ecs_delete(world, e5);
+
+    ecs_query_t *q = ecs_query(world, {
+        .expr = "Position",
+        .order_by = ecs_id(Position),
+        .order_by_callback = compare_position,
+        .flags = EcsQueryMatchEmptyTables
+    });
+
+    ecs_iter_t it = ecs_query_iter(world, q);
+
+    test_assert(ecs_query_next(&it));
+    test_int(it.count, 4);
+
+    test_assert(it.entities[0] == e2);
+    test_assert(it.entities[1] == e4);
+    test_assert(it.entities[2] == e1);
+    test_assert(it.entities[3] == e3);
+    test_assert(!ecs_query_next(&it));
+
+    ecs_query_fini(q);
+
+    ecs_fini(world);
+}
+
+void OrderBy_order_empty_table_only(void) {
+    ecs_world_t *world = ecs_mini();
+
+    ECS_COMPONENT(world, Position);
+
+    // Create empty table
+    ecs_entity_t e = ecs_new_w(world, Position);
+    ecs_delete(world, e);
+
+    ecs_query_t *q = ecs_query(world, {
+        .expr = "Position",
+        .order_by = ecs_id(Position),
+        .order_by_callback = compare_position,
+        .flags = EcsQueryMatchEmptyTables
+    });
+
+    ecs_iter_t it = ecs_query_iter(world, q);
+    test_assert(!ecs_query_next(&it));
+
+    ecs_query_fini(q);
+
+    ecs_fini(world);
+}
diff --git a/test/query/src/main.c b/test/query/src/main.c
@@ -2036,6 +2036,8 @@ void OrderBy_sort_by_wildcard(void);
 void OrderBy_sort_not_term(void);
 void OrderBy_sort_or_term(void);
 void OrderBy_sort_optional_term(void);
+void OrderBy_order_empty_table(void);
+void OrderBy_order_empty_table_only(void);
 
 // Testsuite 'OrderByEntireTable'
 void OrderByEntireTable_sort_by_component(void);
@@ -10066,6 +10068,14 @@ bake_test_case OrderBy_testcases[] = {
     {
         "sort_optional_term",
         OrderBy_sort_optional_term
+    },
+    {
+        "order_empty_table",
+        OrderBy_order_empty_table
+    },
+    {
+        "order_empty_table_only",
+        OrderBy_order_empty_table_only
     }
 };
 
@@ -10584,7 +10594,7 @@ static bake_test_suite suites[] = {
         "OrderBy",
         NULL,
         NULL,
-        42,
+        44,
         OrderBy_testcases
     },
     {