Skip to content

Commit

Permalink
feat(query): support alter table comment (#15227)
Browse files Browse the repository at this point in the history
  • Loading branch information
TCeason authored Apr 13, 2024
1 parent 2071b02 commit 6453c48
Show file tree
Hide file tree
Showing 15 changed files with 242 additions and 0 deletions.
5 changes: 5 additions & 0 deletions src/query/ast/src/ast/format/ast_format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1483,6 +1483,11 @@ impl<'ast> Visitor<'ast> for AstFormatVisitor {
let action_format_ctx = AstFormatContext::new(action_name);
FormatTreeNode::new(action_format_ctx)
}
AlterTableAction::ModifyTableComment { new_comment } => {
let action_name = format!("Action Modify Comment To {}", new_comment);
let action_format_ctx = AstFormatContext::new(action_name);
FormatTreeNode::new(action_format_ctx)
}
AlterTableAction::AddColumn { column, option } => {
let action_name = match option {
AddColumnOption::First => format!("Action Add column {} first", column),
Expand Down
4 changes: 4 additions & 0 deletions src/query/ast/src/ast/format/syntax/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,10 @@ pub(crate) fn pretty_alter_table_action(action: AlterTableAction) -> RcDoc<'stat
AlterTableAction::RenameTable { new_table } => RcDoc::line()
.append(RcDoc::text("RENAME TO "))
.append(RcDoc::text(new_table.to_string())),
AlterTableAction::ModifyTableComment { new_comment } => RcDoc::line()
.append(RcDoc::text("COMMENT='"))
.append(RcDoc::text(new_comment))
.append(RcDoc::text("'")),
AlterTableAction::RenameColumn {
old_column,
new_column,
Expand Down
7 changes: 7 additions & 0 deletions src/query/ast/src/ast/statements/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -365,6 +365,10 @@ pub enum AlterTableAction {
old_column: Identifier,
new_column: Identifier,
},
ModifyTableComment {
#[drive(skip)]
new_comment: String,
},
ModifyColumn {
action: ModifyColumnAction,
},
Expand Down Expand Up @@ -402,6 +406,9 @@ impl Display for AlterTableAction {
AlterTableAction::RenameTable { new_table } => {
write!(f, "RENAME TO {new_table}")?;
}
AlterTableAction::ModifyTableComment { new_comment } => {
write!(f, "COMMENT='{new_comment}'")?;
}
AlterTableAction::RenameColumn {
old_column,
new_column,
Expand Down
7 changes: 7 additions & 0 deletions src/query/ast/src/parser/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3163,6 +3163,12 @@ pub fn alter_table_action(i: Input) -> IResult<AlterTableAction> {
new_column,
},
);
let modify_table_comment = map(
rule! {
COMMENT ~ ^"=" ~ ^#literal_string
},
|(_, _, new_comment)| AlterTableAction::ModifyTableComment { new_comment },
);
let add_column = map(
rule! {
ADD ~ COLUMN? ~ #column_def ~ ( #add_column_option )?
Expand Down Expand Up @@ -3230,6 +3236,7 @@ pub fn alter_table_action(i: Input) -> IResult<AlterTableAction> {
| #drop_table_cluster_key
| #rename_table
| #rename_column
| #modify_table_comment
| #add_column
| #drop_column
| #modify_column
Expand Down
1 change: 1 addition & 0 deletions src/query/ast/tests/it/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,7 @@ fn test_statement() {
r#"OPTIMIZE TABLE t PURGE BEFORE (SNAPSHOT => '9828b23f74664ff3806f44bbc1925ea5') LIMIT 10;"#,
r#"OPTIMIZE TABLE t PURGE BEFORE (TIMESTAMP => '2023-06-26 09:49:02.038483'::TIMESTAMP) LIMIT 10;"#,
r#"ALTER TABLE t CLUSTER BY(c1);"#,
r#"ALTER TABLE t COMMENT='t1-commnet';"#,
r#"ALTER TABLE t DROP CLUSTER KEY;"#,
r#"ALTER TABLE t RECLUSTER FINAL WHERE c1 > 0 LIMIT 10;"#,
r#"ALTER TABLE t ADD c int null;"#,
Expand Down
34 changes: 34 additions & 0 deletions src/query/ast/tests/it/testdata/statement.txt
Original file line number Diff line number Diff line change
Expand Up @@ -10024,6 +10024,40 @@ AlterTable(
)


---------- Input ----------
ALTER TABLE t COMMENT='t1-commnet';
---------- Output ---------
ALTER TABLE t COMMENT='t1-commnet'
---------- AST ------------
AlterTable(
AlterTableStmt {
if_exists: false,
table_reference: Table {
span: Some(
12..13,
),
catalog: None,
database: None,
table: Identifier {
span: Some(
12..13,
),
name: "t",
quote: None,
is_hole: false,
},
alias: None,
temporal: None,
pivot: None,
unpivot: None,
},
action: ModifyTableComment {
new_comment: "t1-commnet",
},
},
)


---------- Input ----------
ALTER TABLE t DROP CLUSTER KEY;
---------- Output ---------
Expand Down
3 changes: 3 additions & 0 deletions src/query/service/src/interpreters/access/privilege_access.rs
Original file line number Diff line number Diff line change
Expand Up @@ -705,6 +705,9 @@ impl AccessChecker for PrivilegeAccess {
Plan::ModifyTableColumn(plan) => {
self.validate_table_access(&plan.catalog, &plan.database, &plan.table, UserPrivilegeType::Alter, false).await?
}
Plan::ModifyTableComment(plan) => {
self.validate_table_access(&plan.catalog, &plan.database, &plan.table, UserPrivilegeType::Alter, false).await?
}
Plan::DropTableColumn(plan) => {
self.validate_table_access(&plan.catalog, &plan.database, &plan.table, UserPrivilegeType::Alter, false).await?
}
Expand Down
3 changes: 3 additions & 0 deletions src/query/service/src/interpreters/interpreter_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,9 @@ impl InterpreterFactory {
ctx,
*set_options.clone(),
)?)),
Plan::ModifyTableComment(new_comment) => Ok(Arc::new(
ModifyTableCommentInterpreter::try_create(ctx, *new_comment.clone())?,
)),
Plan::RenameTableColumn(rename_table_column) => Ok(Arc::new(
RenameTableColumnInterpreter::try_create(ctx, *rename_table_column.clone())?,
)),
Expand Down
105 changes: 105 additions & 0 deletions src/query/service/src/interpreters/interpreter_table_modify_comment.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,105 @@
// Copyright 2021 Datafuse Labs
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

use std::sync::Arc;

use databend_common_catalog::table::TableExt;
use databend_common_exception::ErrorCode;
use databend_common_exception::Result;
use databend_common_meta_app::schema::DatabaseType;
use databend_common_meta_app::schema::UpdateTableMetaReq;
use databend_common_meta_types::MatchSeq;
use databend_common_sql::plans::ModifyTableCommentPlan;
use databend_common_storages_stream::stream_table::STREAM_ENGINE;
use databend_common_storages_view::view_table::VIEW_ENGINE;

use crate::interpreters::Interpreter;
use crate::pipelines::PipelineBuildResult;
use crate::sessions::QueryContext;
use crate::sessions::TableContext;
pub struct ModifyTableCommentInterpreter {
ctx: Arc<QueryContext>,
plan: ModifyTableCommentPlan,
}

impl ModifyTableCommentInterpreter {
pub fn try_create(ctx: Arc<QueryContext>, plan: ModifyTableCommentPlan) -> Result<Self> {
Ok(ModifyTableCommentInterpreter { ctx, plan })
}
}

#[async_trait::async_trait]
impl Interpreter for ModifyTableCommentInterpreter {
fn name(&self) -> &str {
"ModifyTableCommentInterpreter"
}

fn is_ddl(&self) -> bool {
true
}

#[async_backtrace::framed]
async fn execute2(&self) -> Result<PipelineBuildResult> {
let catalog_name = self.plan.catalog.as_str();
let db_name = self.plan.database.as_str();
let tbl_name = self.plan.table.as_str();

let tbl = self
.ctx
.get_catalog(catalog_name)
.await?
.get_table(&self.ctx.get_tenant(), db_name, tbl_name)
.await
.ok();

if let Some(table) = &tbl {
// check mutability
table.check_mutable()?;

let table_info = table.get_table_info();
let engine = table.engine();
if matches!(engine, VIEW_ENGINE | STREAM_ENGINE) {
return Err(ErrorCode::TableEngineNotSupported(format!(
"{}.{} engine is {} that doesn't support alter",
&self.plan.database, &self.plan.table, engine
)));
}
if table_info.db_type != DatabaseType::NormalDB {
return Err(ErrorCode::TableEngineNotSupported(format!(
"{}.{} doesn't support alter",
&self.plan.database, &self.plan.table
)));
}

let catalog = self.ctx.get_catalog(self.plan.catalog.as_str()).await?;
let table_id = table_info.ident.table_id;
let table_version = table_info.ident.seq;
let mut new_table_meta = table_info.meta.clone();
new_table_meta.comment = self.plan.new_comment.clone();

let req = UpdateTableMetaReq {
table_id,
seq: MatchSeq::Exact(table_version),
new_table_meta,
copied_files: None,
deduplicated_label: None,
update_stream_meta: vec![],
};

catalog.update_table_meta(table_info, req).await?;
};

Ok(PipelineBuildResult::create())
}
}
2 changes: 2 additions & 0 deletions src/query/service/src/interpreters/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ mod interpreter_table_index_create;
mod interpreter_table_index_drop;
mod interpreter_table_index_refresh;
mod interpreter_table_modify_column;
mod interpreter_table_modify_comment;
mod interpreter_table_optimize;
mod interpreter_table_recluster;
mod interpreter_table_rename;
Expand Down Expand Up @@ -217,6 +218,7 @@ pub use interpreter_table_index_create::CreateTableIndexInterpreter;
pub use interpreter_table_index_drop::DropTableIndexInterpreter;
pub use interpreter_table_index_refresh::RefreshTableIndexInterpreter;
pub use interpreter_table_modify_column::ModifyTableColumnInterpreter;
pub use interpreter_table_modify_comment::ModifyTableCommentInterpreter;
pub use interpreter_table_optimize::OptimizeTableInterpreter;
pub use interpreter_table_recluster::ReclusterTableInterpreter;
pub use interpreter_table_rename::RenameTableInterpreter;
Expand Down
9 changes: 9 additions & 0 deletions src/query/sql/src/planner/binder/ddl/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ use crate::plans::DropTablePlan;
use crate::plans::ExistsTablePlan;
use crate::plans::ModifyColumnAction as ModifyColumnActionInPlan;
use crate::plans::ModifyTableColumnPlan;
use crate::plans::ModifyTableCommentPlan;
use crate::plans::OptimizeTableAction;
use crate::plans::OptimizeTablePlan;
use crate::plans::Plan;
Expand Down Expand Up @@ -835,6 +836,14 @@ impl Binder {
table,
})))
}
AlterTableAction::ModifyTableComment { new_comment } => {
Ok(Plan::ModifyTableComment(Box::new(ModifyTableCommentPlan {
new_comment: new_comment.to_string(),
catalog,
database,
table,
})))
}
AlterTableAction::RenameColumn {
old_column,
new_column,
Expand Down
1 change: 1 addition & 0 deletions src/query/sql/src/planner/format/display_plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ impl Plan {
Plan::UndropTable(_) => Ok("UndropTable".to_string()),
Plan::DescribeTable(_) => Ok("DescribeTable".to_string()),
Plan::RenameTable(_) => Ok("RenameTable".to_string()),
Plan::ModifyTableComment(_) => Ok("ModifyTableComment".to_string()),
Plan::SetOptions(_) => Ok("SetOptions".to_string()),
Plan::RenameTableColumn(_) => Ok("RenameTableColumn".to_string()),
Plan::AddTableColumn(_) => Ok("AddTableColumn".to_string()),
Expand Down
15 changes: 15 additions & 0 deletions src/query/sql/src/planner/plans/ddl/table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -251,6 +251,21 @@ impl RenameTablePlan {
}
}

/// Modify table comment.
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct ModifyTableCommentPlan {
pub new_comment: String,
pub catalog: String,
pub database: String,
pub table: String,
}

impl ModifyTableCommentPlan {
pub fn schema(&self) -> DataSchemaRef {
Arc::new(DataSchema::empty())
}
}

/// SetOptions
#[derive(Clone, Debug, PartialEq, Eq)]
pub struct SetOptionsPlan {
Expand Down
2 changes: 2 additions & 0 deletions src/query/sql/src/planner/plans/plan.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ use crate::plans::InsertMultiTable;
use crate::plans::KillPlan;
use crate::plans::MergeInto;
use crate::plans::ModifyTableColumnPlan;
use crate::plans::ModifyTableCommentPlan;
use crate::plans::OptimizeTablePlan;
use crate::plans::PresignPlan;
use crate::plans::ReclusterTablePlan;
Expand Down Expand Up @@ -201,6 +202,7 @@ pub enum Plan {
DropTable(Box<DropTablePlan>),
UndropTable(Box<UndropTablePlan>),
RenameTable(Box<RenameTablePlan>),
ModifyTableComment(Box<ModifyTableCommentPlan>),
RenameTableColumn(Box<RenameTableColumnPlan>),
AddTableColumn(Box<AddTableColumnPlan>),
DropTableColumn(Box<DropTableColumnPlan>),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,50 @@ select database,table,name,data_type,comment from system.columns where table='t'
default t c1 VARCHAR 'c1-column'
default t c2 INT 'test'

statement ok
alter table t comment='s1';

query TT
select name, comment from system.tables where name='t' and database='default';
----
t s1

query TT
show create table t;
----
t CREATE TABLE t ( c1 VARCHAR NULL COMMENT 'c1-column', c2 INT NULL COMMENT 'test' ) ENGINE=FUSE COMMENT = 's1'

statement ok
drop table if exists t1;

statement ok
create table t1(id int) comment ='t1-comment';

query TT
show create table t1;
----
t1 CREATE TABLE t1 ( id INT NULL ) ENGINE=FUSE COMMENT = 't1-comment'

query TT
select name, comment from system.tables where name='t1' and database='default';
----
t1 t1-comment

statement ok
alter table t1 comment='t1-new-comment';

query TT
show create table t1;
----
t1 CREATE TABLE t1 ( id INT NULL ) ENGINE=FUSE COMMENT = 't1-new-comment'

query TT
select name, comment from system.tables where name='t1' and database='default';
----
t1 t1-new-comment

statement ok
DROP TABLE IF EXISTS t;

statement ok
DROP TABLE IF EXISTS t1;

0 comments on commit 6453c48

Please sign in to comment.