Skip to content

Commit

Permalink
fix the bug when alter column that doesn't have a default value to no… (
Browse files Browse the repository at this point in the history 
#5105)

* fix the bug when alter column that doesn't have a default value to not nullable

* address comments

Co-authored-by: Sophie <84560950+Sophie-Xie@users.noreply.github.com>
  • Loading branch information
@jievince @Sophie-Xie
jievince and Sophie-Xie committed Dec 26, 2022
1 parent 3540691 commit c2e0ee9
Show file tree
Hide file tree
Showing 3 changed files with 92 additions and 26 deletions.
11 changes: 9 additions & 2 deletions src/graph/validator/MaintainValidator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ namespace graph {
// Validate columns of schema.
// Check validity of columns and fill to thrift structure.
static Status validateColumns(const std::vector<ColumnSpecification *> &columnSpecs,
meta::cpp2::Schema &schema) {
meta::cpp2::Schema &schema,
bool isAlter = false) {
for (auto &spec : columnSpecs) {
meta::cpp2::ColumnDef column;
auto type = spec->type();
Expand Down Expand Up @@ -59,6 +60,12 @@ static Status validateColumns(const std::vector<ColumnSpecification *> &columnSp
if (!column.nullable_ref().has_value()) {
column.nullable_ref() = true;
}
// Should report an error when altering the column
// which doesn't have default value to not nullable
if (isAlter && !column.nullable_ref().value() && !column.default_value_ref().has_value()) {
return Status::SemanticError("Column `%s' must have a default value if it's not nullable",
spec->name()->c_str());
}
schema.columns_ref().value().emplace_back(std::move(column));
}
return Status::OK();
Expand Down Expand Up @@ -92,7 +99,7 @@ static StatusOr<std::vector<meta::cpp2::AlterSchemaItem>> validateSchemaOpts(
return Status::SemanticError("Duplicate column name `%s'", spec->name()->c_str());
}
}
NG_LOG_AND_RETURN_IF_ERROR(validateColumns(specs, schema));
NG_LOG_AND_RETURN_IF_ERROR(validateColumns(specs, schema, true));
}

schemaItem.schema_ref() = std::move(schema);
Expand Down
16 changes: 8 additions & 8 deletions tests/tck/features/schema/Comment.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ Feature: Schema Comment
"""
ALTER TAG test_comment_tag comment = <tag_of_person_comment_modified>;
ALTER TAG test_comment_tag ADD (gender string COMMENT 'The gender.');
ALTER TAG test_comment_tag CHANGE (name string NOT NULL);
ALTER TAG test_comment_tag CHANGE (name string NOT NULL DEFAULT "jack");
ALTER TAG test_comment_tag DROP (age);
"""
Then the execution should be successful
Expand All @@ -134,15 +134,15 @@ Feature: Schema Comment
SHOW CREATE tag test_comment_tag;
"""
Then the result should be, in any order:
| Tag | Create Tag |
| "test_comment_tag" | 'CREATE TAG `test_comment_tag` (\n `name` string NOT NULL,\n `gender` string NULL COMMENT "The gender."\n) ttl_duration = 0, ttl_col = "", comment = <tag_of_person_comment_modified>' |
| Tag | Create Tag |
| "test_comment_tag" | 'CREATE TAG `test_comment_tag` (\n `name` string NOT NULL DEFAULT \"jack\",\n `gender` string NULL COMMENT "The gender."\n) ttl_duration = 0, ttl_col = "", comment = <tag_of_person_comment_modified>' |
When executing query:
"""
DESC tag test_comment_tag;
"""
Then the result should be, in any order:
| Field | Type | Null | Default | Comment |
| "name" | "string" | "NO" | EMPTY | EMPTY |
| "name" | "string" | "NO" | "jack" | EMPTY |
| "gender" | "string" | "YES" | EMPTY | "The gender." |
# tag index
When executing query:
Expand Down Expand Up @@ -188,7 +188,7 @@ Feature: Schema Comment
"""
ALTER EDGE test_comment_edge comment = <edge_of_person_comment_modified>;
ALTER EDGE test_comment_edge ADD (gender string COMMENT 'The gender.');
ALTER EDGE test_comment_edge CHANGE (name string NOT NULL);
ALTER EDGE test_comment_edge CHANGE (name string NOT NULL DEFAULT "jack");
ALTER EDGE test_comment_edge DROP (age);
"""
Then the execution should be successful
Expand All @@ -198,15 +198,15 @@ Feature: Schema Comment
SHOW CREATE edge test_comment_edge;
"""
Then the result should be, in any order:
| Edge | Create Edge |
| "test_comment_edge" | 'CREATE EDGE `test_comment_edge` (\n `name` string NOT NULL,\n `gender` string NULL COMMENT "The gender."\n) ttl_duration = 0, ttl_col = "", comment = <edge_of_person_comment_modified>' |
| Edge | Create Edge |
| "test_comment_edge" | 'CREATE EDGE `test_comment_edge` (\n `name` string NOT NULL DEFAULT \"jack\",\n `gender` string NULL COMMENT "The gender."\n) ttl_duration = 0, ttl_col = "", comment = <edge_of_person_comment_modified>' |
When executing query:
"""
DESC edge test_comment_edge;
"""
Then the result should be, in any order:
| Field | Type | Null | Default | Comment |
| "name" | "string" | "NO" | EMPTY | EMPTY |
| "name" | "string" | "NO" | "jack" | EMPTY |
| "gender" | "string" | "YES" | EMPTY | "The gender." |
# edge index
When executing query:
Expand Down
91 changes: 75 additions & 16 deletions tests/tck/features/schema/Schema.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -516,7 +516,7 @@ Feature: Insert string vid of vertex and edge
"""
ALTER TAG t CHANGE (description string NOT NULL)
"""
Then the execution should be successful
Then a SemanticError should be raised at runtime: Column `description' must have a default value if it's not nullable
And wait 3 seconds
# insert
When executing query:
Expand All @@ -532,13 +532,6 @@ Feature: Insert string vid of vertex and edge
Then the result should be, in any order:
| t.name | t.age | t.description |
| "N/A" | -1 | "some one" |
And wait 3 seconds
# insert without default prop, failed
When executing query:
"""
INSERT VERTEX t() VALUES "1":()
"""
Then a ExecutionError should be raised at runtime: Storage Error: The not null field doesn't have a default value.
# test alter edge with default value
When executing query:
"""
Expand Down Expand Up @@ -569,14 +562,7 @@ Feature: Insert string vid of vertex and edge
"""
ALTER EDGE e CHANGE (description string NOT NULL)
"""
Then the execution should be successful
And wait 3 seconds
# insert without default prop, failed
When executing query:
"""
INSERT EDGE e() VALUES "1"->"2":()
"""
Then a SemanticError should be raised at runtime: The property `description' is not nullable and has no default value.
Then a SemanticError should be raised at runtime: Column `description' must have a default value if it's not nullable
# test alter edge with timestamp default
When executing query:
"""
Expand Down Expand Up @@ -850,6 +836,78 @@ Feature: Insert string vid of vertex and edge
DROP SPACE issue2009;
"""
Then the execution should be successful
Then drop the used space

Scenario: alter a tag to add an column which doesn't have a default value to not nullable
Given an empty graph
And create a space with following options:
| partition_num | 1 |
| replica_factor | 1 |
| vid_type | FIXED_STRING(20) |
When executing query:
"""
CREATE TAG person(age int);
"""
Then the execution should be successful
When executing query:
"""
CREATE TAG INDEX person_age_index ON person(age);
"""
Then the execution should be successful
And wait 3 seconds
When executing query:
"""
INSERT VERTEX person values "1":(23);
"""
Then the execution should be successful
When executing query:
"""
LOOKUP ON person YIELD properties(VERTEX) AS props;
"""
Then the result should be, in any order, with relax comparison:
| props |
| {age: 23} |
When executing query:
"""
ALTER TAG person ADD (gender bool NOT NULL);
"""
Then a SemanticError should be raised at runtime: Column `gender' must have a default value if it's not nullable

Scenario: alter a edge to change an column which doesn't have a default value to not nullable
Given an empty graph
And create a space with following options:
| partition_num | 1 |
| replica_factor | 1 |
| vid_type | FIXED_STRING(20) |
When executing query:
"""
CREATE EDGE person(age int);
"""
Then the execution should be successful
When executing query:
"""
CREATE EDGE INDEX person_age_index ON person(age);
"""
Then the execution should be successful
And wait 3 seconds
When executing query:
"""
INSERT EDGE person values "1"->"2":(23);
"""
Then the execution should be successful
When executing query:
"""
LOOKUP ON person YIELD properties(EDGE) AS props;
"""
Then the result should be, in any order, with relax comparison:
| props |
| {age: 23} |
When executing query:
"""
ALTER EDGE person ADD (gender bool NOT NULL);
"""
Then a SemanticError should be raised at runtime: Column `gender' must have a default value if it's not nullable
Then drop the used space

Scenario: Don't allow DOT in schema name
Given an empty graph
Expand All @@ -862,3 +920,4 @@ Feature: Insert string vid of vertex and edge
CREATE TAG `tag.prop`()
"""
Then a SyntaxError should be raised at runtime: Don't allow DOT in label: near `.prop`()'
Then drop the used space

0 comments on commit c2e0ee9

Please sign in to comment.