From cda2efcd1ecaa5a849a5f135adc951c892578eff Mon Sep 17 00:00:00 2001 From: Dennis Felsing Date: Thu, 7 May 2026 05:33:35 +0000 Subject: [PATCH] audit: add missing audit log entry for ALTER TABLE ADD COLUMN The `Op::AlterAddColumn` handler was missing an audit log entry, unlike every other ALTER operation. This adds an `AlterAddColumnV1` event that records the table ID, column name, column type, and nullability. Introduced in #30470. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/adapter/src/catalog/transact.rs | 22 +++++++++++++++++++ src/audit-log/src/lib.rs | 21 ++++++++++++++++++ src/catalog-protos/objects_hashes.json | 2 +- src/catalog-protos/src/audit_log.rs | 30 +++++++++++++++++++++++--- src/catalog-protos/src/objects.rs | 19 ++++++++++++++++ 5 files changed, 90 insertions(+), 4 deletions(-) diff --git a/src/adapter/src/catalog/transact.rs b/src/adapter/src/catalog/transact.rs index 8ae758ca9c285..a49e60b4666dd 100644 --- a/src/adapter/src/catalog/transact.rs +++ b/src/adapter/src/catalog/transact.rs @@ -984,6 +984,9 @@ impl Catalog { typ, sql, } => { + let column_name = name.to_string(); + let column_type = typ.to_string(); + let nullable = typ.nullable; let mut new_entry = state.get_entry(&id).clone(); let version = new_entry.item.add_column(name, typ, sql)?; // All versions of a table share the same shard, so it shouldn't matter what @@ -998,6 +1001,25 @@ impl Catalog { }; table.collections.insert(version, new_global_id); + if Self::should_audit_log_item(new_entry.item()) { + let details = EventDetails::AlterAddColumnV1(mz_audit_log::AlterAddColumnV1 { + id: id.to_string(), + column: column_name, + column_type, + nullable, + }); + CatalogState::add_to_audit_log( + &state.system_configuration, + oracle_write_ts, + session, + tx, + audit_events, + EventType::Alter, + catalog_type_to_audit_object_type(new_entry.item().typ()), + details, + )?; + } + tx.update_item(id, new_entry.into())?; storage_collections_to_register.insert(new_global_id, shard_id); } diff --git a/src/audit-log/src/lib.rs b/src/audit-log/src/lib.rs index 5880f139b80e7..489ed67758d4f 100644 --- a/src/audit-log/src/lib.rs +++ b/src/audit-log/src/lib.rs @@ -232,6 +232,7 @@ pub enum EventDetails { UpdateItemV1(UpdateItemV1), RenameSchemaV1(RenameSchemaV1), AlterRetainHistoryV1(AlterRetainHistoryV1), + AlterAddColumnV1(AlterAddColumnV1), ToNewIdV1(ToNewIdV1), FromPreviousIdV1(FromPreviousIdV1), SetV1(SetV1), @@ -1080,6 +1081,25 @@ pub struct AlterRetainHistoryV1 { pub new_history: Option, } +#[derive( + Clone, + Debug, + Serialize, + Deserialize, + PartialOrd, + PartialEq, + Eq, + Ord, + Hash, + Arbitrary +)] +pub struct AlterAddColumnV1 { + pub id: String, + pub column: String, + pub column_type: String, + pub nullable: bool, +} + #[derive( Clone, Debug, @@ -1192,6 +1212,7 @@ impl EventDetails { EventDetails::AlterRetainHistoryV1(v) => { serde_json::to_value(v).expect("must serialize") } + EventDetails::AlterAddColumnV1(v) => serde_json::to_value(v).expect("must serialize"), EventDetails::ToNewIdV1(v) => serde_json::to_value(v).expect("must serialize"), EventDetails::FromPreviousIdV1(v) => serde_json::to_value(v).expect("must serialize"), EventDetails::SetV1(v) => serde_json::to_value(v).expect("must serialize"), diff --git a/src/catalog-protos/objects_hashes.json b/src/catalog-protos/objects_hashes.json index 2f3a39c661431..8546b8d992323 100644 --- a/src/catalog-protos/objects_hashes.json +++ b/src/catalog-protos/objects_hashes.json @@ -1,7 +1,7 @@ [ { "name": "objects.rs", - "md5": "dfd89e1c62b7f1663d6429846a76159f" + "md5": "0088761ac51bdbf5ef8be4a031d8e58a" }, { "name": "objects_v74.rs", diff --git a/src/catalog-protos/src/audit_log.rs b/src/catalog-protos/src/audit_log.rs index 8dbdf6253ac3e..a185b821346c3 100644 --- a/src/catalog-protos/src/audit_log.rs +++ b/src/catalog-protos/src/audit_log.rs @@ -14,9 +14,9 @@ //! because of Rust's orphan rules. use mz_audit_log::{ - AlterApplyReplacementV1, AlterDefaultPrivilegeV1, AlterRetainHistoryV1, AlterSetClusterV1, - AlterSourceSinkV1, CreateClusterReplicaV1, CreateClusterReplicaV2, CreateClusterReplicaV3, - CreateClusterReplicaV4, CreateIndexV1, CreateMaterializedViewV1, + AlterAddColumnV1, AlterApplyReplacementV1, AlterDefaultPrivilegeV1, AlterRetainHistoryV1, + AlterSetClusterV1, AlterSourceSinkV1, CreateClusterReplicaV1, CreateClusterReplicaV2, + CreateClusterReplicaV3, CreateClusterReplicaV4, CreateIndexV1, CreateMaterializedViewV1, CreateOrDropClusterReplicaReasonV1, CreateRoleV1, CreateSourceSinkV1, CreateSourceSinkV2, CreateSourceSinkV3, CreateSourceSinkV4, DropClusterReplicaV1, DropClusterReplicaV2, DropClusterReplicaV3, EventDetails, EventType, EventV1, FromPreviousIdV1, FullNameV1, @@ -1188,6 +1188,28 @@ impl RustType for Alte } } +impl RustType for AlterAddColumnV1 { + fn into_proto(&self) -> crate::objects::audit_log_event_v1::AlterAddColumnV1 { + crate::objects::audit_log_event_v1::AlterAddColumnV1 { + id: self.id.to_string(), + column: self.column.clone(), + column_type: self.column_type.clone(), + nullable: self.nullable, + } + } + + fn from_proto( + proto: crate::objects::audit_log_event_v1::AlterAddColumnV1, + ) -> Result { + Ok(AlterAddColumnV1 { + id: proto.id, + column: proto.column, + column_type: proto.column_type, + nullable: proto.nullable, + }) + } +} + impl RustType for ToNewIdV1 { fn into_proto(&self) -> crate::objects::audit_log_event_v1::ToNewIdV1 { crate::objects::audit_log_event_v1::ToNewIdV1 { @@ -1342,6 +1364,7 @@ impl RustType for EventDetails { EventDetails::AlterRetainHistoryV1(details) => { AlterRetainHistoryV1(details.into_proto()) } + EventDetails::AlterAddColumnV1(details) => AlterAddColumnV1(details.into_proto()), EventDetails::ToNewIdV1(details) => ToNewIdV1(details.into_proto()), EventDetails::FromPreviousIdV1(details) => FromPreviousIdV1(details.into_proto()), EventDetails::SetV1(details) => SetV1(details.into_proto()), @@ -1428,6 +1451,7 @@ impl RustType for EventDetails { ResetAllV1(Empty {}) => Ok(EventDetails::ResetAllV1), RotateKeysV1(details) => Ok(EventDetails::RotateKeysV1(details.into_rust()?)), CreateRoleV1(details) => Ok(EventDetails::CreateRoleV1(details.into_rust()?)), + AlterAddColumnV1(details) => Ok(EventDetails::AlterAddColumnV1(details.into_rust()?)), } } } diff --git a/src/catalog-protos/src/objects.rs b/src/catalog-protos/src/objects.rs index 549137dd2f3dd..d9111769ae7a8 100644 --- a/src/catalog-protos/src/objects.rs +++ b/src/catalog-protos/src/objects.rs @@ -2298,6 +2298,24 @@ pub mod audit_log_event_v1 { pub new_history: Option, } + #[derive( + Clone, + Debug, + PartialEq, + Eq, + PartialOrd, + Ord, + Serialize, + Deserialize, + Arbitrary + )] + pub struct AlterAddColumnV1 { + pub id: String, + pub column: String, + pub column_type: String, + pub nullable: bool, + } + #[derive( Clone, Debug, @@ -2490,6 +2508,7 @@ pub mod audit_log_event_v1 { CreateMaterializedViewV1(CreateMaterializedViewV1), AlterApplyReplacementV1(AlterApplyReplacementV1), CreateRoleV1(CreateRoleV1), + AlterAddColumnV1(AlterAddColumnV1), } }