Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add column if not exists #5393

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/operator/src/expr_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::<Result<Vec<AddColumn>>>()?,
Expand Down
28 changes: 23 additions & 5 deletions src/operator/src/statement/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -885,12 +885,13 @@ impl StatementExecutor {
Ok(Output::new_with_affected_rows(0))
}

/// Verifies an alter and returns whether it is necessary to perform the alter.
killme2008 marked this conversation as resolved.
Show resolved Hide resolved
fn verify_alter(
&self,
table_id: TableId,
table_info: Arc<TableInfo>,
expr: AlterTableExpr,
) -> Result<()> {
) -> Result<bool> {
let request: AlterTableRequest = common_grpc_expr::alter_expr_to_request(table_id, expr)
.context(AlterExprToRequestSnafu)?;

Expand All @@ -907,6 +908,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<String> = table_info
.meta
.schema
.column_schemas()
.iter()
.map(|schema| schema.name.clone())
killme2008 marked this conversation as resolved.
Show resolved Hide resolved
.collect();
if columns.iter().all(|column| {
column_names.contains(&column.column_schema.name) && column.add_if_not_exists
}) {
return Ok(false);
}
}

let _ = table_info
Expand All @@ -916,7 +932,7 @@ impl StatementExecutor {
.build()
.context(error::BuildTableMetaSnafu { table_name })?;

Ok(())
Ok(true)
}

#[tracing::instrument(skip_all)]
Expand Down Expand Up @@ -971,8 +987,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(),
Expand Down
1 change: 0 additions & 1 deletion src/query/src/datafusion/planner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,6 @@ impl DfContextProviderAdapter {
);

let tables = resolve_tables(table_names, &mut table_provider).await?;

Ok(Self {
engine_state,
session_state,
Expand Down
2 changes: 2 additions & 0 deletions src/sql/src/parsers/alter_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<AddColumn, ParserError> {
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]);
killme2008 marked this conversation as resolved.
Show resolved Hide resolved
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) {
Expand All @@ -312,6 +313,7 @@ fn parse_add_columns(parser: &mut Parser) -> std::result::Result<AddColumn, Pars
Ok(AddColumn {
column_def,
location,
add_if_not_exists,
})
}

Expand Down
1 change: 1 addition & 0 deletions src/sql/src/statements/alter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ pub enum AlterTableOperation {
pub struct AddColumn {
pub column_def: ColumnDef,
pub location: Option<AddColumnLocation>,
pub add_if_not_exists: bool,
}

impl Display for AddColumn {
Expand Down
12 changes: 12 additions & 0 deletions tests/cases/standalone/common/alter/add_col.result
Original file line number Diff line number Diff line change
Expand Up @@ -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;

+---+-------------------------+---+
Expand Down Expand Up @@ -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;

+--------+----------------------+-----+------+---------+---------------+
Expand Down
6 changes: 6 additions & 0 deletions tests/cases/standalone/common/alter/add_col.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Loading