chore: Remove unneeded create credential message (#2474)

crates/metastore/src/database.rs

@@ -743,37 +743,6 @@ impl State {
                 // Add to creadentials map
                 self.credentials_names.insert(create_credentials.name, oid);
             }
-            Mutation::CreateCredential(create_credential) => {
-                validate_object_name(&create_credential.name)?;
-                if self
-                    .credentials_names
-                    .get(&create_credential.name)
-                    .is_some()
-                {
-                    return Err(MetastoreError::DuplicateName(create_credential.name));
-                }
-
-                // Create new entry
-                let oid = self.next_oid();
-                let ent = CredentialsEntry {
-                    meta: EntryMeta {
-                        entry_type: EntryType::Credentials,
-                        id: oid,
-                        // The credentials, just like databases doesn't have any parent.
-                        parent: DATABASE_PARENT_ID,
-                        name: create_credential.name.clone(),
-                        builtin: false,
-                        external: false,
-                        is_temp: false,
-                    },
-                    options: create_credential.options,
-                    comment: create_credential.comment,
-                };
-                self.entries.insert(oid, CatalogEntry::Credentials(ent))?;
-
-                // Add to creadentials map
-                self.credentials_names.insert(create_credential.name, oid);
-            }
             Mutation::CreateSchema(create_schema) => {
                 validate_object_name(&create_schema.name)?;
 

crates/protogen/proto/metastore/service.proto

@@ -38,9 +38,8 @@ message Mutation {
     CreateCredentials create_credentials = 15;
     DropCredentials drop_credentials = 16;
     UpdateDeploymentStorage update_deployment_storage = 17;
-    CreateCredential create_credential = 18;
   }
-  // next: 18
+  // next: 17
 }
 
 message DropDatabase {
@@ -167,13 +166,6 @@ message CreateCredentials {
   bool or_replace = 4;
 }
 
-message CreateCredential {
-  string name = 1;
-  options.CredentialsOptions options = 2;
-  string comment = 3;
-  bool or_replace = 4;
-}
-
 message DropCredentials {
   string name = 1;
   bool if_exists = 2;

crates/protogen/src/metastore/types/service.rs

@@ -22,7 +22,6 @@ pub enum Mutation {
     DropTunnel(DropTunnel),
     AlterTunnelRotateKeys(AlterTunnelRotateKeys),
     CreateCredentials(CreateCredentials),
-    CreateCredential(CreateCredential),
     DropCredentials(DropCredentials),
     // Deployment metadata updates
     UpdateDeploymentStorage(UpdateDeploymentStorage),
@@ -61,9 +60,6 @@ impl TryFrom<service::mutation::Mutation> for Mutation {
             service::mutation::Mutation::CreateCredentials(v) => {
                 Mutation::CreateCredentials(v.try_into()?)
             }
-            service::mutation::Mutation::CreateCredential(v) => {
-                Mutation::CreateCredential(v.try_into()?)
-            }
             service::mutation::Mutation::DropCredentials(v) => {
                 Mutation::DropCredentials(v.try_into()?)
             }
@@ -100,9 +96,6 @@ impl TryFrom<Mutation> for service::mutation::Mutation {
             Mutation::CreateCredentials(v) => {
                 service::mutation::Mutation::CreateCredentials(v.into())
             }
-            Mutation::CreateCredential(v) => {
-                service::mutation::Mutation::CreateCredential(v.into())
-            }
             Mutation::DropCredentials(v) => service::mutation::Mutation::DropCredentials(v.into()),
             Mutation::UpdateDeploymentStorage(v) => {
                 service::mutation::Mutation::UpdateDeploymentStorage(v.into())
@@ -647,37 +640,6 @@ impl From<CreateCredentials> for service::CreateCredentials {
     }
 }
 
-#[derive(Debug, Clone, Arbitrary, PartialEq, Eq)]
-pub struct CreateCredential {
-    pub name: String,
-    pub options: CredentialsOptions,
-    pub comment: String,
-    pub or_replace: bool,
-}
-
-impl TryFrom<service::CreateCredential> for CreateCredential {
-    type Error = ProtoConvError;
-    fn try_from(value: service::CreateCredential) -> Result<Self, Self::Error> {
-        Ok(CreateCredential {
-            name: value.name,
-            options: value.options.required("options")?,
-            comment: value.comment,
-            or_replace: value.or_replace,
-        })
-    }
-}
-
-impl From<CreateCredential> for service::CreateCredential {
-    fn from(value: CreateCredential) -> Self {
-        service::CreateCredential {
-            name: value.name,
-            options: Some(value.options.into()),
-            comment: value.comment,
-            or_replace: value.or_replace,
-        }
-    }
-}
-
 #[derive(Debug, Clone, Arbitrary, PartialEq, Eq)]
 pub struct DropCredentials {
     pub name: String,

crates/protogen/src/sqlexec/physical_plan.rs

@@ -342,7 +342,7 @@ pub struct AnalyzeExec {
 pub struct ExecutionPlanExtension {
     #[prost(
         oneof = "ExecutionPlanExtensionType",
-        tags = "1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32"
+        tags = "1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31"
     )]
     pub inner: Option<ExecutionPlanExtensionType>,
 }
@@ -416,6 +416,4 @@ pub enum ExecutionPlanExtensionType {
     DataSourceMetricsExecAdapter(DataSourceMetricsExecAdapter),
     #[prost(message, tag = "31")]
     DescribeTable(DescribeTableExec),
-    #[prost(message, tag = "32")]
-    CreateCredentialExec(CreateCredentialExec),
 }

crates/sqlexec/src/extension_codec.rs

@@ -25,7 +25,6 @@ use crate::planner::physical_plan::alter_database::AlterDatabaseExec;
 use crate::planner::physical_plan::alter_table::AlterTableExec;
 use crate::planner::physical_plan::alter_tunnel_rotate_keys::AlterTunnelRotateKeysExec;
 use crate::planner::physical_plan::copy_to::CopyToExec;
-use crate::planner::physical_plan::create_credential::CreateCredentialExec;
 use crate::planner::physical_plan::create_credentials::CreateCredentialsExec;
 use crate::planner::physical_plan::create_external_database::CreateExternalDatabaseExec;
 use crate::planner::physical_plan::create_external_table::CreateExternalTableExec;
@@ -172,18 +171,6 @@ impl<'a> PhysicalExtensionCodec for GlareDBExtensionCodec<'a> {
                     or_replace: create_credentials.or_replace,
                 })
             }
-            proto::ExecutionPlanExtensionType::CreateCredentialExec(create_credential) => {
-                let options = create_credential
-                    .options
-                    .ok_or(DataFusionError::Plan("options is required".to_string()))?;
-                Arc::new(CreateCredentialExec {
-                    name: create_credential.name,
-                    catalog_version: create_credential.catalog_version,
-                    options: options.try_into()?,
-                    comment: create_credential.comment,
-                    or_replace: create_credential.or_replace,
-                })
-            }
             proto::ExecutionPlanExtensionType::DescribeTable(describe_table) => {
                 let entry = describe_table
                     .entry
@@ -542,14 +529,6 @@ impl<'a> PhysicalExtensionCodec for GlareDBExtensionCodec<'a> {
                 schema_reference: Some(exec.schema_reference.clone().into()),
                 if_not_exists: exec.if_not_exists,
             })
-        } else if let Some(exec) = node.as_any().downcast_ref::<CreateCredentialExec>() {
-            proto::ExecutionPlanExtensionType::CreateCredentialExec(proto::CreateCredentialExec {
-                name: exec.name.clone(),
-                catalog_version: exec.catalog_version,
-                options: Some(exec.options.clone().into()),
-                comment: exec.comment.clone(),
-                or_replace: exec.or_replace,
-            })
         } else if let Some(exec) = node.as_any().downcast_ref::<CreateCredentialsExec>() {
             proto::ExecutionPlanExtensionType::CreateCredentialsExec(proto::CreateCredentialsExec {
                 name: exec.name.clone(),

crates/sqlexec/src/parser.rs

@@ -274,24 +274,32 @@ pub struct CreateCredentialsStmt {
     pub comment: String,
     /// replace if it exists
     pub or_replace: bool,
+    /// Whether or not we're using the old syntax CREATE CREDENTIALS. New syntax
+    /// is just CREATE CREDENTIAL.
+    pub deprecated: bool,
 }
 
 impl fmt::Display for CreateCredentialsStmt {
     fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
-        write!(
-            f,
-            "CREATE {or_replace}CREDENTIALS {name} PROVIDER {provider}",
-            or_replace = if self.or_replace { "OR REPLACE " } else { "" },
-            name = self.name,
-            provider = self.provider
-        )?;
+        write!(f, "CREATE ")?;
+        if self.or_replace {
+            write!(f, "OR REPLACE ")?;
+        }
+        if self.deprecated {
+            write!(f, "CREDENTIALS ")?;
+        } else {
+            write!(f, "CREDENTIAL ")?;
+        }
+        write!(f, "{} ", self.name)?;
+        write!(f, "PROVIDER {}", self.provider)?;
+
         if !self.options.is_empty() {
             write!(f, " {}", self.options)?;
         }
-
         if !self.comment.is_empty() {
-            write!(f, " COMMENT '{}'", self.comment)?;
+            write!(f, " COMMENT '{}'", self.comment)?
         }
+
         Ok(())
     }
 }
@@ -416,8 +424,6 @@ pub enum StatementWithExtensions {
     /// Alter tunnel extension.
     AlterTunnel(AlterTunnelStmt),
     /// Create credentials extension.
-    CreateCredential(CreateCredentialStmt),
-    /// Create credentials extension.
     CreateCredentials(CreateCredentialsStmt),
     /// Drop credentials extension.
     DropCredentials(DropCredentialsStmt),
@@ -437,7 +443,6 @@ impl fmt::Display for StatementWithExtensions {
             StatementWithExtensions::CreateTunnel(stmt) => write!(f, "{}", stmt),
             StatementWithExtensions::DropTunnel(stmt) => write!(f, "{}", stmt),
             StatementWithExtensions::AlterTunnel(stmt) => write!(f, "{}", stmt),
-            StatementWithExtensions::CreateCredential(stmt) => write!(f, "{}", stmt),
             StatementWithExtensions::CreateCredentials(stmt) => write!(f, "{}", stmt),
             StatementWithExtensions::DropCredentials(stmt) => write!(f, "{}", stmt),
             StatementWithExtensions::CopyTo(stmt) => write!(f, "{}", stmt),
@@ -772,25 +777,16 @@ impl<'a> CustomParser<'a> {
             "".to_owned()
         };
 
-        let stmt = if deprecated {
-            StatementWithExtensions::CreateCredentials(CreateCredentialsStmt {
+        Ok(StatementWithExtensions::CreateCredentials(
+            CreateCredentialsStmt {
                 name,
                 provider,
                 options,
                 comment,
                 or_replace,
-            })
-        } else {
-            StatementWithExtensions::CreateCredential(CreateCredentialStmt {
-                name,
-                provider,
-                options,
-                comment,
-                or_replace,
-            })
-        };
-
-        Ok(stmt)
+                deprecated,
+            },
+        ))
     }
 
     fn parse_object_type(&mut self, object_type: &str) -> Result<Ident, ParserError> {
@@ -1233,8 +1229,12 @@ mod tests {
     #[test]
     fn create_credentials_roundtrips() {
         let test_cases = [
+            // Deprecated syntax
             "CREATE CREDENTIALS qa PROVIDER debug OPTIONS (table_type = 'never_ending')",
             "CREATE CREDENTIALS qa PROVIDER debug OPTIONS (table_type = 'never_ending') COMMENT 'for debug'",
+            // New syntax
+            "CREATE CREDENTIAL qa PROVIDER debug OPTIONS (table_type = 'never_ending')",
+            "CREATE CREDENTIAL qa PROVIDER debug OPTIONS (table_type = 'never_ending') COMMENT 'for debug'",
         ];
 
         for test_case in test_cases {

crates/sqlexec/src/planner/extension.rs

@@ -7,7 +7,7 @@ use crate::{
 use datafusion::logical_expr::{Extension as LogicalPlanExtension, UserDefinedLogicalNodeCore};
 
 use super::logical_plan::{
-    AlterDatabase, AlterTable, AlterTunnelRotateKeys, CopyTo, CreateCredential, CreateCredentials,
+    AlterDatabase, AlterTable, AlterTunnelRotateKeys, CopyTo, CreateCredentials,
     CreateExternalDatabase, CreateExternalTable, CreateSchema, CreateTable, CreateTempTable,
     CreateTunnel, CreateView, Delete, DescribeTable, DropCredentials, DropDatabase, DropSchemas,
     DropTables, DropTunnel, DropViews, Insert, SetVariable, ShowVariable, Update,
@@ -21,7 +21,6 @@ pub enum ExtensionType {
     AlterDatabase,
     AlterTable,
     AlterTunnelRotateKeys,
-    CreateCredential,
     CreateCredentials,
     CreateExternalDatabase,
     CreateExternalTable,
@@ -52,7 +51,6 @@ impl FromStr for ExtensionType {
             AlterDatabase::EXTENSION_NAME => Self::AlterDatabase,
             AlterTable::EXTENSION_NAME => Self::AlterTable,
             AlterTunnelRotateKeys::EXTENSION_NAME => Self::AlterTunnelRotateKeys,
-            CreateCredential::EXTENSION_NAME => Self::CreateCredential,
             CreateCredentials::EXTENSION_NAME => Self::CreateCredentials,
             CreateExternalDatabase::EXTENSION_NAME => Self::CreateExternalDatabase,
             CreateExternalTable::EXTENSION_NAME => Self::CreateExternalTable,

crates/sqlexec/src/planner/logical_plan/mod.rs

@@ -2,7 +2,6 @@ mod alter_database;
 mod alter_table;
 mod alter_tunnel_rotate_keys;
 mod copy_to;
-mod create_credential;
 mod create_credentials;
 mod create_external_database;
 mod create_external_table;
@@ -48,7 +47,6 @@ pub use alter_database::*;
 pub use alter_table::*;
 pub use alter_tunnel_rotate_keys::*;
 pub use copy_to::*;
-pub use create_credential::*;
 pub use create_credentials::*;
 pub use create_external_database::*;
 pub use create_external_table::*;