From e02a1d6ca7242b8ea3f9cffbb01aca07a47eb8b0 Mon Sep 17 00:00:00 2001
From: EnRaiha <15997552+EnRaiha@users.noreply.github.com>
Date: Sun, 20 Sep 2026 22:37:18 +0800
Subject: [PATCH] fix(pgwire): answer a real class for the arm-less error
 variants

The blanket INTERNAL_ERROR fallback in error_to_sqlstate flattened 65 of the
101 crate::Error variants, so a statement reading a balance, hitting a period
lock, a legal hold, a retention floor, a type guard, or a mirror write got
XX000 while the crate's classify() map already carried the class. The
fallback now borrows it through classify() and answers from the same numeric
table the remote path reads.

The bridge exposed one arm doing harm: RetryableSchemaChanged classified as a
plan error, which reads as 'fix your SQL' for a condition the lease paths
retry. It now classifies as a write conflict (40001), matching its contract.
---
 .../control/server/pgwire/types/error_map.rs  | 110 +++++++++++++++++-
 nodedb/src/error_classify.rs                  |   5 +-
 2 files changed, 113 insertions(+), 2 deletions(-)

diff --git a/nodedb/src/control/server/pgwire/types/error_map.rs b/nodedb/src/control/server/pgwire/types/error_map.rs
index 290e8e11f..aaf19c934 100644
--- a/nodedb/src/control/server/pgwire/types/error_map.rs
+++ b/nodedb/src/control/server/pgwire/types/error_map.rs
@@ -201,7 +201,14 @@ pub fn error_to_sqlstate(err: &crate::Error) -> (&'static str, &'static str, Str
         crate::Error::DataPlane(code) => {
             crate::control::server::shared::ddl::sqlstate::error_code_to_sqlstate(code)
         }
-        _ => ("ERROR", sqlstate::INTERNAL_ERROR, err.to_string()),
+        // A variant with no arm of its own carries its class on the error:
+        // `classify` borrows it (the crate's one Error-to-NodeDbError map),
+        // then this table answers instead of a blanket `XX000`.
+        _ => (
+            "ERROR",
+            numeric_code_to_sqlstate(crate::error_classify::classify(err).code()),
+            err.to_string(),
+        ),
     }
 }
 
@@ -466,4 +473,105 @@ mod tests {
 
         assert!(message.contains("timestamp"));
     }
+
+    /// A variant with no arm of its own still carries a class: the fallback
+    /// borrows it through `classify` and this table answers it.
+    #[test]
+    fn arm_less_variants_keep_their_class_through_the_fallback() {
+        use crate::Error;
+
+        let cases = vec![
+            (
+                Error::AppendOnlyViolation {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::APPEND_ONLY_VIOLATION,
+            ),
+            (
+                Error::BalanceViolation {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::BALANCE_VIOLATION,
+            ),
+            (
+                Error::InsufficientBalance {
+                    collection: "c".into(),
+                    key: "k".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::CHECK_VIOLATION,
+            ),
+            (
+                Error::PeriodLocked {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::PERIOD_LOCKED,
+            ),
+            (
+                Error::RetentionViolation {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::RETENTION_VIOLATION,
+            ),
+            (
+                Error::LegalHoldActive {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::LEGAL_HOLD_ACTIVE,
+            ),
+            (
+                Error::StateTransitionViolation {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::STATE_TRANSITION_VIOLATION,
+            ),
+            (
+                Error::TransitionCheckViolation {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::TRANSITION_CHECK_VIOLATION,
+            ),
+            (
+                Error::TypeGuardViolation {
+                    collection: "c".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::TYPE_GUARD_VIOLATION,
+            ),
+            (
+                Error::TypeMismatch {
+                    collection: "c".into(),
+                    key: "k".into(),
+                    detail: "d".into(),
+                },
+                sqlstate::DATATYPE_MISMATCH,
+            ),
+            (
+                Error::MirrorReadOnly {
+                    database: "db".into(),
+                },
+                sqlstate::READ_ONLY_SQL_TRANSACTION,
+            ),
+            (
+                // Retryable in the lease paths, so the fallback must not read
+                // as a plan/syntax error now that it answers through `classify`.
+                Error::RetryableSchemaChanged {
+                    descriptor: "users".into(),
+                },
+                sqlstate::SERIALIZATION_FAILURE,
+            ),
+        ];
+
+        for (err, expected) in cases {
+            let (_severity, state, _message) = error_to_sqlstate(&err);
+            assert_eq!(state, expected, "{err:?}");
+        }
+    }
 }
diff --git a/nodedb/src/error_classify.rs b/nodedb/src/error_classify.rs
index 1b08e588d..e18f94eff 100644
--- a/nodedb/src/error_classify.rs
+++ b/nodedb/src/error_classify.rs
@@ -177,8 +177,11 @@ pub(crate) fn classify(e: &Error) -> NodeDbError {
         Error::InvalidLimitValue { clause, value } => {
             NodeDbError::invalid_limit_value(*clause, value.clone())
         }
+        // Retryable, like a write conflict: the descriptor-lease paths already
+        // retry this variant, so it must not reach a client as a plan/syntax
+        // error, which reads as "fix your SQL".
         Error::RetryableSchemaChanged { descriptor } => {
-            NodeDbError::plan_error(format!("retryable schema change on {descriptor}"))
+            NodeDbError::write_conflict("catalog", descriptor.clone())
         }
         Error::RetryableLeaderChange {
             group_id,