Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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
6 changes: 3 additions & 3 deletions src/datanode/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,8 @@ pub enum Error {
#[snafu(display("Specified timestamp key or primary key column not found: {}", name))]
KeyColumnNotFound { name: String, backtrace: Backtrace },

#[snafu(display("Invalid primary key: {}", msg))]
InvalidPrimaryKey { msg: String, backtrace: Backtrace },
#[snafu(display("Illegal primary keys definition: {}", msg))]
IllegalPrimaryKeysDef { msg: String, backtrace: Backtrace },

#[snafu(display(
"Constraint in CREATE TABLE statement is not supported yet: {}",
Expand Down Expand Up @@ -362,7 +362,7 @@ impl ErrorExt for Error {
Error::ColumnValuesNumberMismatch { .. }
| Error::InvalidSql { .. }
| Error::KeyColumnNotFound { .. }
| Error::InvalidPrimaryKey { .. }
| Error::IllegalPrimaryKeysDef { .. }
| Error::MissingTimestampColumn { .. }
| Error::CatalogNotFound { .. }
| Error::SchemaNotFound { .. }
Expand Down
18 changes: 9 additions & 9 deletions src/datanode/src/sql/create.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ use table::requests::*;

use crate::error::{
self, CatalogNotFoundSnafu, CatalogSnafu, ConstraintNotSupportedSnafu, CreateSchemaSnafu,
CreateTableSnafu, InsertSystemCatalogSnafu, InvalidPrimaryKeySnafu, KeyColumnNotFoundSnafu,
CreateTableSnafu, IllegalPrimaryKeysDefSnafu, InsertSystemCatalogSnafu, KeyColumnNotFoundSnafu,
RegisterSchemaSnafu, Result, SchemaExistsSnafu, SchemaNotFoundSnafu,
};
use crate::sql::SqlHandler;
Expand Down Expand Up @@ -149,8 +149,8 @@ impl SqlHandler {

ensure!(
pk_map.len() < 2,
InvalidPrimaryKeySnafu {
msg: "Multiple definitions of primary key found"
IllegalPrimaryKeysDefSnafu {
msg: "not allowed to inline multiple primary keys in columns options"
}
);

Expand Down Expand Up @@ -181,8 +181,8 @@ impl SqlHandler {
}
} else if is_primary {
if !primary_keys.is_empty() {
return InvalidPrimaryKeySnafu {
msg: "Multiple definitions of primary key found",
return IllegalPrimaryKeysDefSnafu {
msg: "found definitions of primary keys in multiple places",
}
.fail();
}
Expand Down Expand Up @@ -213,7 +213,7 @@ impl SqlHandler {

ensure!(
!primary_keys.iter().any(|index| *index == ts_index),
InvalidPrimaryKeySnafu {
IllegalPrimaryKeysDefSnafu {
msg: "time index column can't be included in primary key"
}
);
Expand Down Expand Up @@ -327,7 +327,7 @@ mod tests {
let error = handler
.create_to_request(42, parsed_stmt, &TableReference::bare("demo_table"))
.unwrap_err();
assert_matches!(error, Error::InvalidPrimaryKey { .. });
assert_matches!(error, Error::IllegalPrimaryKeysDef { .. });
}

#[tokio::test]
Expand All @@ -342,7 +342,7 @@ mod tests {
let error = handler
.create_to_request(42, parsed_stmt, &TableReference::bare("demo_table"))
.unwrap_err();
assert_matches!(error, Error::InvalidPrimaryKey { .. });
assert_matches!(error, Error::IllegalPrimaryKeysDef { .. });
}

#[tokio::test]
Expand Down Expand Up @@ -400,7 +400,7 @@ mod tests {
let error = handler
.create_to_request(42, create_table, &TableReference::full("c", "s", "demo"))
.unwrap_err();
assert_matches!(error, Error::InvalidPrimaryKey { .. });
assert_matches!(error, Error::IllegalPrimaryKeysDef { .. });
}

#[tokio::test]
Expand Down
6 changes: 5 additions & 1 deletion src/frontend/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -342,6 +342,9 @@ pub enum Error {
#[snafu(backtrace)]
source: query::error::Error,
},

#[snafu(display("Illegal primary keys definition: {}", msg))]
IllegalPrimaryKeysDef { msg: String, backtrace: Backtrace },
}

pub type Result<T> = std::result::Result<T, Error>;
Expand All @@ -352,7 +355,8 @@ impl ErrorExt for Error {
Error::ParseAddr { .. }
| Error::InvalidSql { .. }
| Error::InvalidInsertRequest { .. }
| Error::ColumnValuesNumberMismatch { .. } => StatusCode::InvalidArguments,
| Error::ColumnValuesNumberMismatch { .. }
| Error::IllegalPrimaryKeysDef { .. } => StatusCode::InvalidArguments,

Error::NotSupported { .. } => StatusCode::Unsupported,

Expand Down
46 changes: 41 additions & 5 deletions src/frontend/src/expr_factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,13 +22,14 @@ use datanode::instance::sql::table_idents_to_full_name;
use datatypes::schema::ColumnSchema;
use session::context::QueryContextRef;
use snafu::{ensure, ResultExt};
use sql::ast::{ColumnDef, TableConstraint};
use sql::ast::{ColumnDef, ColumnOption, TableConstraint};
use sql::statements::column_def_to_schema;
use sql::statements::create::{CreateTable, TIME_INDEX};

use crate::error::{
self, BuildCreateExprOnInsertionSnafu, ColumnDataTypeSnafu,
ConvertColumnDefaultConstraintSnafu, InvalidSqlSnafu, ParseSqlSnafu, Result,
ConvertColumnDefaultConstraintSnafu, IllegalPrimaryKeysDefSnafu, InvalidSqlSnafu,
ParseSqlSnafu, Result,
};

pub type CreateExprFactoryRef = Arc<dyn CreateExprFactory + Send + Sync>;
Expand Down Expand Up @@ -88,7 +89,7 @@ pub(crate) fn create_to_expr(
desc: "".to_string(),
column_defs: columns_to_expr(&create.columns, &time_index)?,
time_index,
primary_keys: find_primary_keys(&create.constraints)?,
primary_keys: find_primary_keys(&create.columns, &create.constraints)?,
create_if_not_exists: create.if_not_exists,
// TODO(LFC): Fill in other table options.
table_options: HashMap::from([("engine".to_string(), create.engine.clone())]),
Expand All @@ -98,8 +99,32 @@ pub(crate) fn create_to_expr(
Ok(expr)
}

fn find_primary_keys(constraints: &[TableConstraint]) -> Result<Vec<String>> {
let primary_keys = constraints
fn find_primary_keys(
columns: &[ColumnDef],
constraints: &[TableConstraint],
) -> Result<Vec<String>> {
let columns_pk = columns
.iter()
.filter_map(|x| {
if x.options
.iter()
.any(|o| matches!(o.option, ColumnOption::Unique { is_primary: true }))
{
Some(x.name.value.clone())
} else {
None
}
})
.collect::<Vec<String>>();

ensure!(
columns_pk.len() <= 1,
IllegalPrimaryKeysDefSnafu {
msg: "not allowed to inline multiple primary keys in columns options"
}
);

let constraints_pk = constraints
.iter()
.filter_map(|constraint| match constraint {
TableConstraint::Unique {
Expand All @@ -111,6 +136,17 @@ fn find_primary_keys(constraints: &[TableConstraint]) -> Result<Vec<String>> {
})
.flatten()
.collect::<Vec<String>>();

ensure!(
columns_pk.is_empty() || constraints_pk.is_empty(),
IllegalPrimaryKeysDefSnafu {
msg: "found definitions of primary keys in multiple places"
}
);

let mut primary_keys = Vec::with_capacity(columns_pk.len() + constraints_pk.len());
primary_keys.extend(columns_pk);
primary_keys.extend(constraints_pk);
Ok(primary_keys)
}

Expand Down
90 changes: 0 additions & 90 deletions tests/cases/distributed/create/create.result

This file was deleted.

40 changes: 0 additions & 40 deletions tests/cases/distributed/create/create.sql

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ Affected Rows: 1

CREATE TABLE test_multiple_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE, PRIMARY KEY(host));

Error: 1004(InvalidArguments), Invalid primary key: Multiple definitions of primary key found
Error: 1004(InvalidArguments), Illegal primary keys definition: found definitions of primary keys in multiple places

CREATE TABLE test_multiple_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE, PRIMARY KEY(host), PRIMARY KEY(host));

Error: 1004(InvalidArguments), Invalid primary key: Multiple definitions of primary key found
Error: 1004(InvalidArguments), Illegal primary keys definition: found definitions of primary keys in multiple places

CREATE TABLE test_multiple_inline_pk_definitions (timestamp BIGINT TIME INDEX, host STRING PRIMARY KEY, value DOUBLE PRIMARY KEY);

Error: 1004(InvalidArguments), Invalid primary key: Multiple definitions of primary key found
Error: 1004(InvalidArguments), Illegal primary keys definition: not allowed to inline multiple primary keys in columns options

2 changes: 2 additions & 0 deletions tests/cases/standalone/order/order_variable_size_payload.sql
Original file line number Diff line number Diff line change
Expand Up @@ -123,3 +123,5 @@ DROP table test7;
DROP table test8;

DROP TABLE DirectReports;

-- TODO(LFC): Seems creating distributed table has some column schema related issues, look into "order_variable_size_payload" test case.