diff --git a/src/operator/src/expr_factory.rs b/src/operator/src/expr_factory.rs index e2f19efbd4d3..3acf23ae3878 100644 --- a/src/operator/src/expr_factory.rs +++ b/src/operator/src/expr_factory.rs @@ -507,8 +507,7 @@ pub(crate) fn to_alter_table_expr( Ok(AddColumn { column_def: Some(column_def), location: add_column.location.as_ref().map(From::from), - // TODO(yingwen): We don't support `IF NOT EXISTS` for `ADD COLUMN` yet. - add_if_not_exists: false, + add_if_not_exists: add_column.add_if_not_exists, }) }) .collect::>>()?, diff --git a/src/operator/src/statement/ddl.rs b/src/operator/src/statement/ddl.rs index a6f2e6c1c2d5..23cba11de8da 100644 --- a/src/operator/src/statement/ddl.rs +++ b/src/operator/src/statement/ddl.rs @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::sync::Arc; use api::helper::ColumnDataTypeWrapper; @@ -885,12 +885,17 @@ impl StatementExecutor { Ok(Output::new_with_affected_rows(0)) } + /// Verifies an alter and returns whether it is necessary to perform the alter. + /// + /// # Returns + /// + /// Returns true if the alter need to be porformed; otherwise, it returns false. fn verify_alter( &self, table_id: TableId, table_info: Arc, expr: AlterTableExpr, - ) -> Result<()> { + ) -> Result { let request: AlterTableRequest = common_grpc_expr::alter_expr_to_request(table_id, expr) .context(AlterExprToRequestSnafu)?; @@ -907,6 +912,21 @@ impl StatementExecutor { violated: format!("Invalid table name: {}", new_table_name) } ); + } else if let AlterKind::AddColumns { columns } = alter_kind { + // If all the columns are marked as add_if_not_exists and they already exist in the table, + // there is no need to perform the alter. + let column_names: HashSet<_> = table_info + .meta + .schema + .column_schemas() + .iter() + .map(|schema| &schema.name) + .collect(); + if columns.iter().all(|column| { + column_names.contains(&column.column_schema.name) && column.add_if_not_exists + }) { + return Ok(false); + } } let _ = table_info @@ -916,7 +936,7 @@ impl StatementExecutor { .build() .context(error::BuildTableMetaSnafu { table_name })?; - Ok(()) + Ok(true) } #[tracing::instrument(skip_all)] @@ -971,8 +991,10 @@ impl StatementExecutor { })?; let table_id = table.table_info().ident.table_id; - self.verify_alter(table_id, table.table_info(), expr.clone())?; - + let need_alter = self.verify_alter(table_id, table.table_info(), expr.clone())?; + if !need_alter { + return Ok(Output::new_with_affected_rows(0)); + } info!( "Table info before alter is {:?}, expr: {:?}", table.table_info(), diff --git a/src/query/src/datafusion/planner.rs b/src/query/src/datafusion/planner.rs index c3e8e1f5442f..7927ba16854a 100644 --- a/src/query/src/datafusion/planner.rs +++ b/src/query/src/datafusion/planner.rs @@ -70,7 +70,6 @@ impl DfContextProviderAdapter { ); let tables = resolve_tables(table_names, &mut table_provider).await?; - Ok(Self { engine_state, session_state, diff --git a/src/sql/src/parsers/alter_parser.rs b/src/sql/src/parsers/alter_parser.rs index 4456d28b7591..511caee0f47f 100644 --- a/src/sql/src/parsers/alter_parser.rs +++ b/src/sql/src/parsers/alter_parser.rs @@ -291,6 +291,7 @@ fn parse_string_options(parser: &mut Parser) -> std::result::Result<(String, Str fn parse_add_columns(parser: &mut Parser) -> std::result::Result { parser.expect_keyword(Keyword::ADD)?; let _ = parser.parse_keyword(Keyword::COLUMN); + let add_if_not_exists = parser.parse_keywords(&[Keyword::IF, Keyword::NOT, Keyword::EXISTS]); let mut column_def = parser.parse_column_def()?; column_def.name = ParserContext::canonicalize_identifier(column_def.name); let location = if parser.parse_keyword(Keyword::FIRST) { @@ -312,6 +313,7 @@ fn parse_add_columns(parser: &mut Parser) -> std::result::Result { + assert_eq!(alter.table_name.0[0].value, "test"); + assert_matches!( + alter.alter_operation, + AlterTableOperation::AddColumns { .. } + ); + match alter.alter_operation { + AlterTableOperation::AddColumns { add_columns } => { + let expected = vec![ + AddColumn { + column_def: ColumnDef { + name: Ident::new("a"), + data_type: DataType::Integer(None), + collation: None, + options: vec![], + }, + location: None, + add_if_not_exists: true, + }, + AddColumn { + column_def: ColumnDef { + name: Ident::new("b"), + data_type: DataType::String(None), + collation: None, + options: vec![], + }, + location: None, + add_if_not_exists: false, + }, + AddColumn { + column_def: ColumnDef { + name: Ident::new("c"), + data_type: DataType::Int(None), + collation: None, + options: vec![], + }, + location: None, + add_if_not_exists: true, + }, + ]; + for (idx, add_column) in add_columns.into_iter().enumerate() { + assert_eq!(add_column, expected[idx]); + } + } + _ => unreachable!(), + } + } + _ => unreachable!(), + } + } + #[test] fn test_parse_alter_drop_column() { let sql = "ALTER TABLE my_metric_1 DROP a"; diff --git a/src/sql/src/statements/alter.rs b/src/sql/src/statements/alter.rs index ed44c944de4e..6fcf48d21eaa 100644 --- a/src/sql/src/statements/alter.rs +++ b/src/sql/src/statements/alter.rs @@ -89,6 +89,7 @@ pub enum AlterTableOperation { pub struct AddColumn { pub column_def: ColumnDef, pub location: Option, + pub add_if_not_exists: bool, } impl Display for AddColumn { diff --git a/tests/cases/standalone/common/alter/add_col.result b/tests/cases/standalone/common/alter/add_col.result index b2c7049eff1b..8d951e9419a3 100644 --- a/tests/cases/standalone/common/alter/add_col.result +++ b/tests/cases/standalone/common/alter/add_col.result @@ -19,6 +19,10 @@ ALTER TABLE test ADD COLUMN k INTEGER; Affected Rows: 0 +ALTER TABLE test ADD COLUMN IF NOT EXISTS k INTEGER; + +Affected Rows: 0 + SELECT * FROM test; +---+-------------------------+---+ @@ -108,6 +112,14 @@ ALTER TABLE test ADD COLUMN "foo" STRING default 'foo' PRIMARY KEY, ADD COLUMN " Affected Rows: 0 +ALTER TABLE test ADD COLUMN IF NOT EXISTS "foo" STRING default 'foo' PRIMARY KEY, ADD COLUMN "bar" STRING default 'bar' PRIMARY KEY; + +Error: 4003(TableColumnExists), Column bar already exists in table test + +ALTER TABLE test ADD COLUMN IF NOT EXISTS "foo" STRING default 'foo' PRIMARY KEY, ADD COLUMN IF NOT EXISTS "bar" STRING default 'bar' PRIMARY KEY; + +Affected Rows: 0 + DESC TABLE test; +--------+----------------------+-----+------+---------+---------------+ diff --git a/tests/cases/standalone/common/alter/add_col.sql b/tests/cases/standalone/common/alter/add_col.sql index 7a868a85084b..b666bf24cdad 100644 --- a/tests/cases/standalone/common/alter/add_col.sql +++ b/tests/cases/standalone/common/alter/add_col.sql @@ -6,6 +6,8 @@ INSERT INTO test VALUES (1, 1), (2, 2); ALTER TABLE test ADD COLUMN k INTEGER; +ALTER TABLE test ADD COLUMN IF NOT EXISTS k INTEGER; + SELECT * FROM test; DESC TABLE test; @@ -28,6 +30,10 @@ DESC TABLE test; ALTER TABLE test ADD COLUMN "foo" STRING default 'foo' PRIMARY KEY, ADD COLUMN "bar" STRING default 'bar' PRIMARY KEY; +ALTER TABLE test ADD COLUMN IF NOT EXISTS "foo" STRING default 'foo' PRIMARY KEY, ADD COLUMN "bar" STRING default 'bar' PRIMARY KEY; + +ALTER TABLE test ADD COLUMN IF NOT EXISTS "foo" STRING default 'foo' PRIMARY KEY, ADD COLUMN IF NOT EXISTS "bar" STRING default 'bar' PRIMARY KEY; + DESC TABLE test; DROP TABLE test;