From 123431c615a1eb3605136083bc04856f1cef3678 Mon Sep 17 00:00:00 2001
From: Madeline Shortt <maddi.shortt@gmail.com>
Date: Wed, 8 Jul 2020 15:44:59 -0700
Subject: [PATCH] Fix order by to properly handle lexicographical ordering of
 stringified numbers (#47)

Co-authored-by: Madeline Shortt <mshortt@slack-corp.com>
---
 src/Query/Query.php        |  4 +++-
 tests/SelectClauseTest.php | 11 +++++++++++
 tests/SharedSetup.php      | 33 +++++++++++++++++++++++++++++++++
 3 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/src/Query/Query.php b/src/Query/Query.php
index 4960e8b..3ef93a4 100644
--- a/src/Query/Query.php
+++ b/src/Query/Query.php
@@ -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;
diff --git a/tests/SelectClauseTest.php b/tests/SelectClauseTest.php
index fff8eea..fe97c97 100644
--- a/tests/SelectClauseTest.php
+++ b/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> {
diff --git a/tests/SharedSetup.php b/tests/SharedSetup.php
index 6a3ee21..f2919f8 100644
--- a/tests/SharedSetup.php
+++ b/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[