Skip to content

Commit

Permalink
Fix crash when sorted query with MatchEmptyTables flag only matches e…
Browse files Browse the repository at this point in the history
…mpty tables

Summary: This issue fixes an assert that occurs when a query that is created with `EcsQueryMatchEmptyTables` (which is the default for HSR) and has an `order_by` callback, only matches empty tables.

Reviewed By: Jason-M-Fugate

Differential Revision: D66028916
  • Loading branch information
SanderMertens authored and facebook-github-bot committed Nov 15, 2024
1 parent 544cccc commit 6006fd7
Show file tree
Hide file tree
Showing 5 changed files with 89 additions and 2 deletions.
6 changes: 6 additions & 0 deletions distr/flecs.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 6 additions & 0 deletions src/query/engine/cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
4 changes: 3 additions & 1 deletion test/query/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
63 changes: 63 additions & 0 deletions test/query/src/OrderBy.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
12 changes: 11 additions & 1 deletion test/query/src/main.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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
}
};

Expand Down Expand Up @@ -10584,7 +10594,7 @@ static bake_test_suite suites[] = {
"OrderBy",
NULL,
NULL,
42,
44,
OrderBy_testcases
},
{
Expand Down

0 comments on commit 6006fd7

Please sign in to comment.