Fix order by to properly handle lexicographical ordering of stringifi…

…ed numbers (#47)

Co-authored-by: Madeline Shortt <[email protected]>

src/Query/Query.php

@@ -66,7 +66,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;

tests/SelectClauseTest.php

@@ -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> {

tests/SharedSetup.php

@@ -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;
@@ -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[