From f5ace60850e4e48ccbc006c76894df00b3b3f7f9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ren=C3=A9=20Canna=C3=B2?= <rene.cannao@gmail.com>
Date: Sun, 22 Mar 2026 12:51:03 +0100
Subject: [PATCH] Address review on PgSQL error classifier (PR #5514)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Fix: 57014 (query_canceled) is now classified as REPORT_TO_CLIENT
  instead of FATAL (it's a normal cancellation, not a server crash)
- Fix docstring: crash_shutdown is 57P02 (class 57), not class 58;
  class 58 is system/I/O errors
- Add test for 57014 exception (+1 test, plan 25→26)
---
 include/PgSQLErrorClassifier.h                       |  5 +++--
 lib/PgSQLErrorClassifier.cpp                         | 12 +++++++++---
 .../tap/tests/unit/pgsql_error_classifier_unit-t.cpp |  7 +++++--
 3 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/include/PgSQLErrorClassifier.h b/include/PgSQLErrorClassifier.h
index 1be6b1e6e..471ddce40 100644
--- a/include/PgSQLErrorClassifier.h
+++ b/include/PgSQLErrorClassifier.h
@@ -27,8 +27,9 @@ enum PgSQLErrorAction {
  * - "08" (connection exception): retryable
  * - "40" (transaction rollback, including serialization failure): retryable
  * - "53" (insufficient resources, e.g. too_many_connections): retryable
- * - "57" (operator intervention, e.g. admin_shutdown): fatal
- * - "58" (system error, e.g. crash_shutdown): fatal
+ * - "57" (operator intervention, e.g. admin_shutdown, crash_shutdown): fatal
+ *        Exception: "57014" (query_canceled) is non-fatal
+ * - "58" (system error, e.g. I/O error): fatal
  * - All others (syntax, constraint, etc.): report to client
  *
  * @param sqlstate  5-character SQLSTATE string (e.g., "08006", "42P01").
diff --git a/lib/PgSQLErrorClassifier.cpp b/lib/PgSQLErrorClassifier.cpp
index e6279e369..cd48afcce 100644
--- a/lib/PgSQLErrorClassifier.cpp
+++ b/lib/PgSQLErrorClassifier.cpp
@@ -26,10 +26,16 @@ PgSQLErrorAction classify_pgsql_error(const char *sqlstate) {
 	// Insufficient resources (too many connections) — retryable
 	if (strcmp(cls, "53") == 0) return PGSQL_ERROR_RETRY;
 
-	// Operator intervention (admin shutdown, crash) — fatal
-	if (strcmp(cls, "57") == 0) return PGSQL_ERROR_FATAL;
+	// Operator intervention — mostly fatal, except query_canceled
+	if (strcmp(cls, "57") == 0) {
+		// 57014 = query_canceled — not fatal, report to client
+		if (strlen(sqlstate) >= 5 && strncmp(sqlstate, "57014", 5) == 0) {
+			return PGSQL_ERROR_REPORT_TO_CLIENT;
+		}
+		return PGSQL_ERROR_FATAL;  // admin_shutdown, crash_shutdown, etc.
+	}
 
-	// System error (I/O error, crash shutdown) — fatal
+	// System error (I/O error) — fatal
 	if (strcmp(cls, "58") == 0) return PGSQL_ERROR_FATAL;
 
 	// Everything else (syntax, constraints, data, etc.) — report to client
diff --git a/test/tap/tests/unit/pgsql_error_classifier_unit-t.cpp b/test/tap/tests/unit/pgsql_error_classifier_unit-t.cpp
index 7d0299096..396492415 100644
--- a/test/tap/tests/unit/pgsql_error_classifier_unit-t.cpp
+++ b/test/tap/tests/unit/pgsql_error_classifier_unit-t.cpp
@@ -43,6 +43,9 @@ static void test_fatal_errors() {
 		"57P02 (crash shutdown): fatal");
 	ok(classify_pgsql_error("58000") == PGSQL_ERROR_FATAL,
 		"58000 (system error): fatal");
+	// 57014 is an exception — query_canceled is NOT fatal
+	ok(classify_pgsql_error("57014") == PGSQL_ERROR_REPORT_TO_CLIENT,
+		"57014 (query canceled): not fatal, report to client");
 }
 
 static void test_non_retryable_errors() {
@@ -81,7 +84,7 @@ static void test_retry_conditions() {
 }
 
 int main() {
-	plan(25);
+	plan(26);
 	int rc = test_init_minimal();
 	ok(rc == 0, "test_init_minimal() succeeds");
 
@@ -92,7 +95,7 @@ int main() {
 	test_non_retryable_errors(); // 5
 	test_edge_cases();           // 3
 	test_retry_conditions();     // 5
-	// Total: 1+3+2+2+4+5+3+5 = 25
+	// Total: 1+3+2+2+5+5+3+5 = 26
 
 	test_cleanup_minimal();
 	return exit_status();