Skip to content

Commit

Permalink
fix: flush table panic when table has interval column close #3235 (#5422
Browse files Browse the repository at this point in the history
)

* fix: flash table panic when table has interval column close #3235

Signed-off-by: yihong0618 <[email protected]>

* Revert "fix: flash table panic when table has interval column close #3235"

This reverts commit ffc63ef.

* fix: create table do not support interval type for now close #3235

Signed-off-by: yihong0618 <[email protected]>

* fix: sqlness

Signed-off-by: yihong0618 <[email protected]>

* fix: address comments

Signed-off-by: yihong0618 <[email protected]>

* fix: address comments fix conflict and more tests

Signed-off-by: yihong0618 <[email protected]>

* fix: address final comments drop useless sqlness tests

Signed-off-by: yihong0618 <[email protected]>

---------

Signed-off-by: yihong0618 <[email protected]>
  • Loading branch information
yihong0618 authored Jan 25, 2025
1 parent 7c5ead9 commit adb5c37
Show file tree
Hide file tree
Showing 9 changed files with 180 additions and 394 deletions.
33 changes: 33 additions & 0 deletions src/operator/src/expr_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -281,10 +281,31 @@ pub fn validate_create_expr(create: &CreateTableExpr) -> Result<()> {
}
.fail();
}
// verify do not contain interval type column issue #3235
for column in &create.column_defs {
if is_interval_type(&column.data_type()) {
return InvalidSqlSnafu {
err_msg: format!(
"column name `{}` is interval type, which is not supported",
column.name
),
}
.fail();
}
}

Ok(())
}

fn is_interval_type(data_type: &ColumnDataType) -> bool {
matches!(
data_type,
ColumnDataType::IntervalYearMonth
| ColumnDataType::IntervalDayTime
| ColumnDataType::IntervalMonthDayNano
)
}

fn find_primary_keys(
columns: &[SqlColumn],
constraints: &[TableConstraint],
Expand Down Expand Up @@ -504,6 +525,12 @@ pub(crate) fn to_alter_table_expr(
)
.map_err(BoxedError::new)
.context(ExternalSnafu)?;
if is_interval_type(&column_def.data_type()) {
return NotSupportedSnafu {
feat: "Add column with interval type",
}
.fail();
}
Ok(AddColumn {
column_def: Some(column_def),
location: add_column.location.as_ref().map(From::from),
Expand All @@ -521,6 +548,12 @@ pub(crate) fn to_alter_table_expr(
let (target_type, target_type_extension) = ColumnDataTypeWrapper::try_from(target_type)
.map(|w| w.to_parts())
.context(ColumnDataTypeSnafu)?;
if is_interval_type(&target_type) {
return NotSupportedSnafu {
feat: "Modify column type to interval type",
}
.fail();
}
AlterTableKind::ModifyColumnTypes(ModifyColumnTypes {
modify_column_types: vec![ModifyColumnType {
column_name: column_name.value,
Expand Down
10 changes: 10 additions & 0 deletions tests/cases/standalone/common/alter/alter_table.result
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ ALTER TABLE test_alt_table ADD COLUMN m INTEGER;

Affected Rows: 0

-- Should fail issue #5422
ALTER TABLE test_alt_table ADD COLUMN n interval;

Error: 1001(Unsupported), Not supported: Add column with interval type

-- Should fail issue #5422
ALTER TABLE test_alt_table MODIFY COLUMN m interval;

Error: 1001(Unsupported), Not supported: Modify column type to interval type

DESC TABLE test_alt_table;

+--------+----------------------+-----+------+---------+---------------+
Expand Down
6 changes: 6 additions & 0 deletions tests/cases/standalone/common/alter/alter_table.sql
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ SELECT * FROM test_alt_table WHERE i = 1;
-- SQLNESS ARG restart=true
ALTER TABLE test_alt_table ADD COLUMN m INTEGER;

-- Should fail issue #5422
ALTER TABLE test_alt_table ADD COLUMN n interval;

-- Should fail issue #5422
ALTER TABLE test_alt_table MODIFY COLUMN m interval;

DESC TABLE test_alt_table;

DROP TABLE test_alt_table;
Expand Down
70 changes: 0 additions & 70 deletions tests/cases/standalone/common/flow/flow_basic.result
Original file line number Diff line number Diff line change
Expand Up @@ -411,76 +411,6 @@ DROP TABLE out_distinct_basic;

Affected Rows: 0

-- test interprete interval
CREATE TABLE numbers_input_basic (
number INT,
ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
PRIMARY KEY(number),
TIME INDEX(ts)
);

Affected Rows: 0

create table out_num_cnt_basic (
a INTERVAL,
b INTERVAL,
c INTERVAL,
ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP TIME INDEX
);

Affected Rows: 0

CREATE FLOW filter_numbers_basic SINK TO out_num_cnt_basic AS
SELECT
INTERVAL '1 day 1 second',
INTERVAL '1 month 1 day 1 second',
INTERVAL '1 year 1 month'
FROM
numbers_input_basic
where
number > 10;

Affected Rows: 0

SHOW CREATE FLOW filter_numbers_basic;

+----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+
| Flow | Create Flow |
+----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+
| filter_numbers_basic | CREATE FLOW IF NOT EXISTS filter_numbers_basic |
| | SINK TO out_num_cnt_basic |
| | AS SELECT INTERVAL '1 day 1 second', INTERVAL '1 month 1 day 1 second', INTERVAL '1 year 1 month' FROM numbers_input_basic WHERE number > 10 |
+----------------------+----------------------------------------------------------------------------------------------------------------------------------------------+

SHOW CREATE TABLE out_num_cnt_basic;

+-------------------+-----------------------------------------------------------+
| Table | Create Table |
+-------------------+-----------------------------------------------------------+
| out_num_cnt_basic | CREATE TABLE IF NOT EXISTS "out_num_cnt_basic" ( |
| | "a" INTERVAL NULL, |
| | "b" INTERVAL NULL, |
| | "c" INTERVAL NULL, |
| | "ts" TIMESTAMP(3) NOT NULL DEFAULT current_timestamp(), |
| | TIME INDEX ("ts") |
| | ) |
| | |
| | ENGINE=mito |
| | |
+-------------------+-----------------------------------------------------------+

drop flow filter_numbers_basic;

Affected Rows: 0

drop table out_num_cnt_basic;

Affected Rows: 0

drop table numbers_input_basic;

Affected Rows: 0

CREATE TABLE bytes_log (
byte INT,
ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
Expand Down
35 changes: 0 additions & 35 deletions tests/cases/standalone/common/flow/flow_basic.sql
Original file line number Diff line number Diff line change
Expand Up @@ -172,41 +172,6 @@ DROP TABLE distinct_basic;

DROP TABLE out_distinct_basic;

-- test interprete interval
CREATE TABLE numbers_input_basic (
number INT,
ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
PRIMARY KEY(number),
TIME INDEX(ts)
);

create table out_num_cnt_basic (
a INTERVAL,
b INTERVAL,
c INTERVAL,
ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP TIME INDEX
);

CREATE FLOW filter_numbers_basic SINK TO out_num_cnt_basic AS
SELECT
INTERVAL '1 day 1 second',
INTERVAL '1 month 1 day 1 second',
INTERVAL '1 year 1 month'
FROM
numbers_input_basic
where
number > 10;

SHOW CREATE FLOW filter_numbers_basic;

SHOW CREATE TABLE out_num_cnt_basic;

drop flow filter_numbers_basic;

drop table out_num_cnt_basic;

drop table numbers_input_basic;

CREATE TABLE bytes_log (
byte INT,
ts TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
Expand Down
19 changes: 0 additions & 19 deletions tests/cases/standalone/common/insert/insert_wrong_type.result

This file was deleted.

8 changes: 0 additions & 8 deletions tests/cases/standalone/common/insert/insert_wrong_type.sql

This file was deleted.

Loading

0 comments on commit adb5c37

Please sign in to comment.