Skip to content

Commit

Permalink
Modify default ColumnOption from NOT NULL to NULL (gluesql#997)
Browse files Browse the repository at this point in the history
For better user experience, Let's modify default ColumnDef.nullable from false to true

CREATE TABLE Items (id INT);
  • Loading branch information
devgony committed Nov 15, 2022
1 parent fa060b3 commit 12afef7
Show file tree
Hide file tree
Showing 6 changed files with 30 additions and 12 deletions.
11 changes: 11 additions & 0 deletions core/src/data/row.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub enum RowError {
#[error("lack of required column: {0}")]
LackOfRequiredColumn(String),

#[error("wrong column name: {0}")]
WrongColumnName(String),

#[error("column and values not matched")]
ColumnAndValuesNotMatched,

Expand Down Expand Up @@ -63,6 +66,14 @@ impl Row {
return Err(RowError::TooManyValues.into());
}

if let Some(wrong_column_name) = columns.iter().find(|column_name| {
!column_defs
.iter()
.any(|column_def| &&column_def.name == column_name)
}) {
return Err(RowError::WrongColumnName(wrong_column_name.to_owned()).into());
}

let columns = if columns.is_empty() {
Columns::All(column_defs.iter().map(|ColumnDef { name, .. }| name))
} else {
Expand Down
7 changes: 4 additions & 3 deletions core/src/translate/ddl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,9 +59,10 @@ pub fn translate_column_def(sql_column_def: &SqlColumnDef) -> Result<ColumnDef>
..
} = sql_column_def;

let nullable = options
.iter()
.any(|SqlColumnOptionDef { option, .. }| option == &SqlColumnOption::Null);
let nullable = !options.iter().any(|SqlColumnOptionDef { option, .. }| {
option == &SqlColumnOption::NotNull
|| option == &SqlColumnOption::Unique { is_primary: true }
});

Ok(ColumnDef {
name: name.value.to_owned(),
Expand Down
2 changes: 1 addition & 1 deletion test-suite/src/alter/alter_table.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ test_case!(alter_table_add_drop, async move {
("INSERT INTO Foo VALUES (1), (2);", Ok(Payload::Insert(2))),
("SELECT * FROM Foo;", Ok(select!(id; I64; 1; 2))),
(
"ALTER TABLE Foo ADD COLUMN amount INTEGER",
"ALTER TABLE Foo ADD COLUMN amount INTEGER NOT NULL",
Err(AlterTableError::DefaultValueRequired(ColumnDef {
name: "amount".to_owned(),
data_type: DataType::Int,
Expand Down
2 changes: 1 addition & 1 deletion test-suite/src/nullable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ test_case!(nullable, async move {
"
CREATE TABLE Test (
id INTEGER NULL,
num INTEGER,
num INTEGER NOT NULL,
name TEXT
)"
);
Expand Down
2 changes: 1 addition & 1 deletion test-suite/src/validate/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use {

test_case!(types, async move {
run!("CREATE TABLE TableB (id BOOLEAN);");
run!("CREATE TABLE TableC (uid INTEGER, null_val INTEGER NULL);");
run!("CREATE TABLE TableC (uid INTEGER NOT NULL, null_val INTEGER NULL);");
run!("INSERT INTO TableB VALUES (FALSE);");
run!("INSERT INTO TableC VALUES (1, NULL);");

Expand Down
18 changes: 12 additions & 6 deletions test-suite/src/values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@ use {
};

test_case!(values, async move {
run!("CREATE TABLE TableA (id INTEGER);");
run!("INSERT INTO TableA (id) VALUES (1);");
run!("INSERT INTO TableA (id) VALUES (9);");
run!("CREATE TABLE Items (id INTEGER NOT NULL, name TEXT, status TEXT DEFAULT 'ACTIVE' NOT NULL);");

let test_cases = [
(
Expand Down Expand Up @@ -162,15 +160,23 @@ test_case!(values, async move {
Err(FetchError::TooManyColumnAliases("Derived".into(), 2, 3).into()),
),
(
"INSERT INTO TableA (id2) VALUES (1);",
"INSERT INTO Items (id) VALUES (1);",
Ok(Payload::Insert(1))
),
(
"INSERT INTO Items (id2) VALUES (1);",
Err(RowError::WrongColumnName("id2".to_owned()).into()),
),
(
"INSERT INTO Items (name) VALUES ('glue');",
Err(RowError::LackOfRequiredColumn("id".to_owned()).into()),
),
(
"INSERT INTO TableA (id) VALUES ('test2', 3)",
"INSERT INTO Items (id) VALUES (3, 'sql')",
Err(RowError::ColumnAndValuesNotMatched.into()),
),
(
"INSERT INTO TableA VALUES (100), (100, 200);",
"INSERT INTO Items VALUES (100, 'a', 'b', 1);",
Err(RowError::TooManyValues.into()),
),
(
Expand Down

0 comments on commit 12afef7

Please sign in to comment.