refactor: unify strong typed object id

Author: wenym1

Cargo.lock

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-type SecretId = u32;
+type SecretId = risingwave_pb::id::SecretId;
 
 mod secret_manager;
 pub use secret_manager::*;

src/common/secret/src/lib.rs

@@ -81,7 +81,7 @@ impl LocalSecretManager {
         let mut secret_guard = self.secrets.write();
         if secret_guard.insert(secret_id, secret).is_some() {
             tracing::error!(
-                secret_id = secret_id,
+                secret_id = %secret_id,
                 "adding a secret but it already exists, overwriting it"
             );
         };
@@ -91,7 +91,7 @@ impl LocalSecretManager {
         let mut secret_guard = self.secrets.write();
         if secret_guard.insert(secret_id, secret).is_none() {
             tracing::error!(
-                secret_id = secret_id,
+                secret_id = %secret_id,
                 "updating a secret but it does not exist, adding it"
             );
         }
@@ -146,15 +146,15 @@ impl LocalSecretManager {
     }
 
     pub fn fill_secret(&self, secret_ref: PbSecretRef) -> SecretResult<String> {
-        let secret_guard: RwLockReadGuard<'_, parking_lot::RawRwLock, HashMap<u32, Vec<u8>>> =
+        let secret_guard: RwLockReadGuard<'_, parking_lot::RawRwLock, HashMap<SecretId, Vec<u8>>> =
             self.secrets.read();
         self.fill_secret_inner(secret_ref, &secret_guard)
     }
 
     fn fill_secret_inner(
         &self,
         secret_ref: PbSecretRef,
-        secret_guard: &RwLockReadGuard<'_, parking_lot::RawRwLock, HashMap<u32, Vec<u8>>>,
+        secret_guard: &RwLockReadGuard<'_, parking_lot::RawRwLock, HashMap<SecretId, Vec<u8>>>,
     ) -> SecretResult<String> {
         let secret_id = secret_ref.secret_id;
         let pb_secret_bytes = secret_guard

src/common/secret/src/secret_manager.rs

@@ -39,7 +39,9 @@ pub use schema::{Field, FieldDisplay, FieldLike, Schema, test_utils as schema_te
 use crate::array::DataChunk;
 pub use crate::constants::hummock;
 use crate::error::BoxedError;
-pub use crate::id::{DatabaseId, SchemaId, TableId};
+pub use crate::id::{
+    ConnectionId, DatabaseId, FunctionId, IndexId, ObjectId, SchemaId, SecretId, TableId,
+};
 
 /// The global version of the catalog.
 pub type CatalogVersion = u64;
@@ -148,8 +150,6 @@ pub trait SysCatalogReader: Sync + Send + 'static {
 
 pub type SysCatalogReaderRef = Arc<dyn SysCatalogReader>;
 
-pub type ObjectId = u32;
-
 #[derive(Clone, Debug, PartialEq, Default, Copy)]
 pub struct TableOption {
     pub retention_seconds: Option<u32>, // second
@@ -178,75 +178,6 @@ impl TableOption {
     }
 }
 
-#[derive(Clone, Copy, Debug, Display, Default, Hash, PartialOrd, PartialEq, Eq)]
-#[display("{index_id}")]
-pub struct IndexId {
-    pub index_id: u32,
-}
-
-impl IndexId {
-    pub const fn new(index_id: u32) -> Self {
-        IndexId { index_id }
-    }
-
-    /// Sometimes the id field is filled later, we use this value for better debugging.
-    pub const fn placeholder() -> Self {
-        IndexId {
-            index_id: OBJECT_ID_PLACEHOLDER,
-        }
-    }
-
-    pub fn index_id(&self) -> u32 {
-        self.index_id
-    }
-}
-
-impl From<u32> for IndexId {
-    fn from(id: u32) -> Self {
-        Self::new(id)
-    }
-}
-impl From<IndexId> for u32 {
-    fn from(id: IndexId) -> Self {
-        id.index_id
-    }
-}
-
-#[derive(Clone, Copy, Debug, Display, Default, Hash, PartialOrd, PartialEq, Eq, Ord)]
-pub struct FunctionId(pub u32);
-
-impl FunctionId {
-    pub const fn new(id: u32) -> Self {
-        FunctionId(id)
-    }
-
-    pub const fn placeholder() -> Self {
-        FunctionId(OBJECT_ID_PLACEHOLDER)
-    }
-
-    pub fn function_id(&self) -> u32 {
-        self.0
-    }
-}
-
-impl From<u32> for FunctionId {
-    fn from(id: u32) -> Self {
-        Self::new(id)
-    }
-}
-
-impl From<&u32> for FunctionId {
-    fn from(id: &u32) -> Self {
-        Self::new(*id)
-    }
-}
-
-impl From<FunctionId> for u32 {
-    fn from(id: FunctionId) -> Self {
-        id.0
-    }
-}
-
 #[derive(Clone, Copy, Debug, Display, Default, Hash, PartialOrd, PartialEq, Eq, Ord)]
 #[display("{user_id}")]
 pub struct UserId {
@@ -283,76 +214,6 @@ impl From<UserId> for u32 {
     }
 }
 
-#[derive(Clone, Copy, Debug, Display, Default, Hash, PartialOrd, PartialEq, Eq, Ord)]
-pub struct ConnectionId(pub u32);
-
-impl ConnectionId {
-    pub const fn new(id: u32) -> Self {
-        ConnectionId(id)
-    }
-
-    pub const fn placeholder() -> Self {
-        ConnectionId(OBJECT_ID_PLACEHOLDER)
-    }
-
-    pub fn connection_id(&self) -> u32 {
-        self.0
-    }
-}
-
-impl From<u32> for ConnectionId {
-    fn from(id: u32) -> Self {
-        Self::new(id)
-    }
-}
-
-impl From<&u32> for ConnectionId {
-    fn from(id: &u32) -> Self {
-        Self::new(*id)
-    }
-}
-
-impl From<ConnectionId> for u32 {
-    fn from(id: ConnectionId) -> Self {
-        id.0
-    }
-}
-
-#[derive(Clone, Copy, Debug, Display, Default, Hash, PartialOrd, PartialEq, Eq, Ord)]
-pub struct SecretId(pub u32);
-
-impl SecretId {
-    pub const fn new(id: u32) -> Self {
-        SecretId(id)
-    }
-
-    pub const fn placeholder() -> Self {
-        SecretId(OBJECT_ID_PLACEHOLDER)
-    }
-
-    pub fn secret_id(&self) -> u32 {
-        self.0
-    }
-}
-
-impl From<u32> for SecretId {
-    fn from(id: u32) -> Self {
-        Self::new(id)
-    }
-}
-
-impl From<&u32> for SecretId {
-    fn from(id: &u32) -> Self {
-        Self::new(*id)
-    }
-}
-
-impl From<SecretId> for u32 {
-    fn from(id: SecretId) -> Self {
-        id.0
-    }
-}
-
 #[derive(Default, Debug, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum ConflictBehavior {
     #[default]

src/common/src/catalog/mod.rs

@@ -100,7 +100,7 @@ pub struct SinkFormatDesc {
     pub options: BTreeMap<String, String>,
     pub secret_refs: BTreeMap<String, PbSecretRef>,
     pub key_encode: Option<SinkEncode>,
-    pub connection_id: Option<u32>,
+    pub connection_id: Option<ConnectionId>,
 }
 
 /// TODO: consolidate with [`crate::source::SourceFormat`] and [`crate::parser::ProtocolProperties`].
@@ -384,7 +384,7 @@ impl SinkCatalog {
             properties: self.properties.clone(),
             sink_type: self.sink_type.to_proto() as i32,
             format_desc: self.format_desc.as_ref().map(|f| f.to_proto()),
-            connection_id: self.connection_id.map(|id| id.into()),
+            connection_id: self.connection_id,
             initialized_at_epoch: self.initialized_at_epoch.map(|e| e.0),
             created_at_epoch: self.created_at_epoch.map(|e| e.0),
             db_name: self.db_name.clone(),
@@ -496,7 +496,7 @@ impl From<PbSink> for SinkCatalog {
             owner: pb.owner.into(),
             sink_type: SinkType::from_proto(sink_type),
             format_desc,
-            connection_id: pb.connection_id.map(ConnectionId),
+            connection_id: pb.connection_id,
             created_at_epoch: pb.created_at_epoch.map(Epoch::from),
             initialized_at_epoch: pb.initialized_at_epoch.map(Epoch::from),
             db_name: pb.db_name,

src/connector/src/sink/catalog/mod.rs

@@ -16,6 +16,7 @@ use std::collections::{BTreeMap, HashMap};
 use std::marker::PhantomData;
 use std::time::Duration;
 
+use risingwave_pb::id::SecretId;
 use risingwave_pb::secret::PbSecretRef;
 
 use crate::error::ConnectorResult;
@@ -264,12 +265,12 @@ impl WithOptionsSecResolved {
         &mut self,
         update_alter_props: BTreeMap<String, String>,
         update_alter_secret_refs: BTreeMap<String, PbSecretRef>,
-    ) -> ConnectorResult<(Vec<u32>, Vec<u32>)> {
+    ) -> ConnectorResult<(Vec<SecretId>, Vec<SecretId>)> {
         let to_add_secret_dep = update_alter_secret_refs
             .values()
             .map(|new_rely_secret| new_rely_secret.secret_id)
             .collect();
-        let mut to_remove_secret_dep: Vec<u32> = vec![];
+        let mut to_remove_secret_dep: Vec<SecretId> = vec![];
 
         // make sure the key in update_alter_props and update_alter_secret_refs not collide
         for key in update_alter_props.keys() {

src/connector/src/with_options.rs

@@ -24,7 +24,6 @@ use risingwave_common::catalog::INFORMATION_SCHEMA_SCHEMA_NAME;
 use risingwave_common::types::{DataType, MapType};
 use risingwave_expr::aggregate::AggType;
 use risingwave_expr::window_function::WindowFuncKind;
-use risingwave_pb::user::grant_privilege::PbObject;
 use risingwave_sqlparser::ast::{
     self, Expr as AstExpr, Function, FunctionArg, FunctionArgExpr, FunctionArgList, Ident,
     OrderByExpr, Statement, Window,
@@ -178,12 +177,7 @@ impl Binder {
                 // record the dependency upon the UDF
                 referred_udfs.insert(func.id);
                 self.check_privilege(
-                    ObjectCheckItem::new(
-                        func.owner,
-                        AclMode::Execute,
-                        func.name.clone(),
-                        PbObject::FunctionId(func.id.function_id()),
-                    ),
+                    ObjectCheckItem::new(func.owner, AclMode::Execute, func.name.clone(), func.id),
                     self.database_id,
                 )?;
 
@@ -232,12 +226,7 @@ impl Binder {
             // record the dependency upon the UDF
             referred_udfs.insert(func.id);
             self.check_privilege(
-                ObjectCheckItem::new(
-                    func.owner,
-                    AclMode::Execute,
-                    func.name.clone(),
-                    PbObject::FunctionId(func.id.function_id()),
-                ),
+                ObjectCheckItem::new(func.owner, AclMode::Execute, func.name.clone(), func.id),
                 self.database_id,
             )?;
             Some(func.clone())

src/frontend/src/binder/expr/function/mod.rs

@@ -60,6 +60,7 @@ pub use relation::{
 };
 // Re-export common types
 pub use risingwave_common::gap_fill::FillStrategy;
+use risingwave_common::id::ObjectId;
 pub use select::{BoundDistinct, BoundSelect};
 pub use set_expr::*;
 pub use statement::BoundStatement;
@@ -69,7 +70,7 @@ pub use values::BoundValues;
 use crate::catalog::catalog_service::CatalogReadGuard;
 use crate::catalog::root_catalog::SchemaPath;
 use crate::catalog::schema_catalog::SchemaCatalog;
-use crate::catalog::{CatalogResult, DatabaseId, TableId, ViewId};
+use crate::catalog::{CatalogResult, DatabaseId, ViewId};
 use crate::error::ErrorCode;
 use crate::session::{AuthContext, SessionImpl, StagingCatalogManager, TemporarySourceManager};
 use crate::user::user_service::UserInfoReadGuard;
@@ -127,7 +128,7 @@ pub struct Binder {
     shared_views: HashMap<ViewId, ShareId>,
 
     /// The included relations while binding a query.
-    included_relations: HashSet<TableId>,
+    included_relations: HashSet<ObjectId>,
 
     /// The included user-defined functions while binding a query.
     included_udfs: HashSet<FunctionId>,
@@ -317,7 +318,7 @@ impl Binder {
     /// After the plan is built, the referenced relations may be changed. We cannot rely on the
     /// collection result of plan, because we still need to record the dependencies that have been
     /// optimised away.
-    pub fn included_relations(&self) -> &HashSet<TableId> {
+    pub fn included_relations(&self) -> &HashSet<ObjectId> {
         &self.included_relations
     }