Skip to content

Commit

Permalink
Fix order by to properly handle lexicographical ordering of stringifi…
Browse files Browse the repository at this point in the history
…ed numbers (#47)

Co-authored-by: Madeline Shortt <[email protected]>
  • Loading branch information
2 people authored and Scott Sandler committed Jul 8, 2020
1 parent 647017d commit 123431c
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 1 deletion.
4 changes: 3 additions & 1 deletion src/Query/Query.php
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,9 @@ protected function applyOrderBy(AsyncMysqlConnection $conn, dataset $data): data
: 1;
} else {
return (
((string)$value_a < (string)$value_b ? 1 : 0) ^ (($rule['direction'] === SortDirection::DESC) ? 1 : 0)
// Use string comparison explicity to handle lexicographical ordering of things like '125' < '5'
(((Str\compare((string)$value_a, (string)$value_b)) < 0) ? 1 : 0) ^
(($rule['direction'] === SortDirection::DESC) ? 1 : 0)
)
? -1
: 1;
Expand Down
11 changes: 11 additions & 0 deletions tests/SelectClauseTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,17 @@ final class SelectClauseTest extends HackTest {
dict['group_id' => 12345, 'table_4_id' => 1000],
],
);

$results = await $conn->query('select id, position from table6 ORDER BY position ASC');
expect($results->rows())->toBeSame(
vec[
dict['id' => 1001, 'position' => '125'],
dict['id' => 1004, 'position' => '25'],
dict['id' => 1000, 'position' => '5'],
dict['id' => 1003, 'position' => '625'],
dict['id' => 1002, 'position' => '75'],
],
);
}

public async function testOrderByNotInSelect(): Awaitable<void> {
Expand Down
33 changes: 33 additions & 0 deletions tests/SharedSetup.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,13 @@ final class SharedSetup {
dict['table_3_id' => 2, 'table_4_id' => 1000, 'group_id' => 12345, 'description' => 'association 3'],
dict['table_3_id' => 3, 'table_4_id' => 1003, 'group_id' => 0, 'description' => 'association 4'],
],
'table6' => vec[
dict['id' => 1000, 'position' => '5'],
dict['id' => 1001, 'position' => '125'],
dict['id' => 1002, 'position' => '75'],
dict['id' => 1003, 'position' => '625'],
dict['id' => 1004, 'position' => '25'],
],
];

$conn->getServer()->databases['db2'] = $database;
Expand Down Expand Up @@ -330,6 +337,32 @@ final class SharedSetup {
),
],
),
'table6' => shape(
'name' => 'table6',
'fields' => vec[
shape(
'name' => 'id',
'type' => DataType::BIGINT,
'length' => 20,
'null' => false,
'hack_type' => 'int',
),
shape(
'name' => 'position',
'type' => DataType::VARCHAR,
'length' => 255,
'null' => false,
'hack_type' => 'string',
),
],
'indexes' => vec[
shape(
'name' => 'PRIMARY',
'type' => 'PRIMARY',
'fields' => keyset['id'],
)
],
),
'association_table' => shape(
'name' => 'association_table',
'fields' => vec[
Expand Down

0 comments on commit 123431c

Please sign in to comment.