Merge pull request #33572 from petrosagg/sources-refactor-part2

storage: quality of life refactors (part 2)

Author: petrosagg

src/adapter/src/error.rs

@@ -519,9 +519,19 @@ impl AdapterError {
             AdapterError::SafeModeViolation(_) => SqlState::INTERNAL_ERROR,
             AdapterError::SubscribeOnlyTransaction => SqlState::INVALID_TRANSACTION_STATE,
             AdapterError::Optimizer(e) => match e {
-                OptimizerError::PlanError(e) => {
-                    AdapterError::PlanError(e.clone()).code() // Delegate to outer
+                OptimizerError::PlanError(PlanError::InvalidSchemaName) => {
+                    SqlState::INVALID_SCHEMA_NAME
                 }
+                OptimizerError::PlanError(PlanError::ColumnAlreadyExists { .. }) => {
+                    SqlState::DUPLICATE_COLUMN
+                }
+                OptimizerError::PlanError(PlanError::UnknownParameter(_)) => {
+                    SqlState::UNDEFINED_PARAMETER
+                }
+                OptimizerError::PlanError(PlanError::ParameterNotAllowed(_)) => {
+                    SqlState::UNDEFINED_PARAMETER
+                }
+                OptimizerError::PlanError(_) => SqlState::INTERNAL_ERROR,
                 OptimizerError::RecursionLimitError(e) => {
                     AdapterError::RecursionLimit(e.clone()).code() // Delegate to outer
                 }
@@ -836,9 +846,9 @@ impl fmt::Display for AdapterError {
 
 impl From<anyhow::Error> for AdapterError {
     fn from(e: anyhow::Error) -> AdapterError {
-        match e.downcast_ref::<PlanError>() {
-            Some(plan_error) => AdapterError::PlanError(plan_error.clone()),
-            None => AdapterError::Unstructured(e),
+        match e.downcast::<PlanError>() {
+            Ok(plan_error) => AdapterError::PlanError(plan_error),
+            Err(e) => AdapterError::Unstructured(e),
         }
     }
 }

src/postgres-util/src/replication.rs

@@ -140,9 +140,11 @@ pub async fn drop_replication_slots(
     Ok(())
 }
 
-pub async fn get_timeline_id(replication_client: &Client) -> Result<u64, PostgresError> {
-    if let Some(r) = simple_query_opt(replication_client, "IDENTIFY_SYSTEM").await? {
-        r.get("timeline")
+pub async fn get_timeline_id(client: &Client) -> Result<u64, PostgresError> {
+    if let Some(r) =
+        simple_query_opt(client, "SELECT timeline_id FROM pg_control_checkpoint()").await?
+    {
+        r.get("timeline_id")
             .expect("Returns a row with a timeline ID")
             .parse::<u64>()
             .map_err(|err| {

src/sql/src/plan/error.rs

@@ -48,7 +48,7 @@ use crate::pure::error::{
 };
 use crate::session::vars::VarError;
 
-#[derive(Clone, Debug)]
+#[derive(Debug)]
 pub enum PlanError {
     /// This feature is not yet supported, but may be supported at some point in the future.
     Unsupported {

src/sql/src/plan/explain/text.rs

@@ -250,7 +250,7 @@ impl HirRelationExpr {
                 }
                 // We only print the offset if it is not trivial, i.e., not 0.
                 let offset_literal = offset.clone().try_into_literal_int64();
-                if !offset_literal.clone().is_ok_and(|offset| offset == 0) {
+                if !offset_literal.as_ref().is_ok_and(|&offset| offset == 0) {
                     let offset = if offset.contains_parameters() {
                         // If we are still before parameter binding, then we can't reduce it to a
                         // literal, so just print the expression.