Skip to content

Commit

Permalink
Fix error of inserting vector with type of integer element in HTTP API (
Browse files Browse the repository at this point in the history
#1088)

### What problem does this PR solve?

Fix error response is given, when using HTTP API to insert integer
element type vector.

Issue link:#1085

### Type of change

- [x] Bug Fix (non-breaking change which fixes an issue)
- [x] Test cases

---------

Signed-off-by: Jin Hai <haijin.chn@gmail.com>
  • Loading branch information
JinHai-CN authored Apr 23, 2024
1 parent dcb5ed2 commit 5b83f4d
Show file tree
Hide file tree
Showing 8 changed files with 178 additions and 55 deletions.
7 changes: 6 additions & 1 deletion docs/references/http_api_reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -431,7 +431,12 @@ Drops an index.
```
curl --request DELETE \
--url localhost:23820/databases/{database_name}/tables/{table_name}/indexes/{index_name} \
--header 'accept: application/json'
--header 'accept: application/json' \
--header 'content-type: application/json' \
--data ' \
{
"drop_option": "ignore_if_not_exists"
} '
```

#### Response
Expand Down
9 changes: 9 additions & 0 deletions python/infinity/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,15 @@ class ErrorCode(IntEnum):
MUTIPLE_FUNCTION_MATCHED = 3064,
INSERT_WITHOUT_VALUES = 3065,
INVALID_CONFLICT_TYPE = 3066,
INVALID_JSON_FORMAT = 3067,
DUPLICATE_COLUMN_NAME = 3068,
INVALID_EXPRESSION = 3069,
SEGMENT_NOT_EXIST = 3070,
AGGREGATE_FUNCTION_WITH_EMPTY_ARGS = 3071,
BLOCK_NOT_EXIST = 3072,
INVALID_TOPK_TYPE = 3073,
INVALID_CREATE_OPTION = 3074,
INVALID_DROP_OPTION = 3075,

TXN_ROLLBACK = 4001,
TXN_CONFLICT = 4002,
Expand Down
11 changes: 2 additions & 9 deletions python/test_http_api/httpapibase.py
Original file line number Diff line number Diff line change
Expand Up @@ -209,17 +209,10 @@ def show_table_columns(self, db_name, table_name, expect):
# part index
def create_index(self, db_name, table_name, index_name, fields=[], index={}, expect={
"error_code": 0
}, opt="kIgnore"):
}, opt="ignore_if_exists"):
url = f"databases/{db_name}/tables/{table_name}/indexes/{index_name}"
ignore = False
if opt == "kIgnore":
ignore = True
elif opt == "kError":
ignore = False
else:
ignore = opt
h = self.set_up_header(['accept', 'content-type'], )
d = self.set_up_data([], {"fields": fields, "index": index, "create_option": {"ignore_if_exists": ignore}})
d = self.set_up_data([], {"fields": fields, "index": index, "create_option": opt})
r = self.request(url, "post", h, d)
self.tear_down(r, expect)
return
Expand Down
4 changes: 2 additions & 2 deletions python/test_http_api/test_index.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ def test_http_create_drop_vector_index_invalid_options(self, column_name, index_
{"type": "double"}, {"type": "varchar"}, {"type": "boolean"},
{"type": "vector", "dimension": 3, "element_type": "float", }])
def test_http_create_drop_different_fulltext_index_invalid_options(self, column_name, index_type,
params, types):
params, types):
db_name = "default"
table_name = "test_create_drop_different_fulltext_index_invalid_options"
idxname = "my_index"
Expand Down Expand Up @@ -661,7 +661,7 @@ def test_http_create_index_with_valid_options(self):
"type": "integer",
}
})
idx_option = ["kError", "kIgnore"]
idx_option = ["error", "ignore_if_exists"]
for opt in idx_option:
self.create_index(db_name, table_name, idxname, ["c1"], {
"type": "HNSW",
Expand Down
9 changes: 4 additions & 5 deletions python/test_http_api/test_insert.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ def test_http_insert_basic(self):
self.insert(db_name, table_name, [{"c1": 1, "c2": 1}])
self.insert(db_name, table_name, [{"c1": 2, "c2": 2}])
self.insert(db_name, table_name, [
{"c1": 3, "c2": 3}, {"c1": 4, "c2": 4}])
{"c1": 3, "c2": 3}, {"c1": 4, "c2": 4}])

self.drop_table(db_name, table_name)
return
Expand Down Expand Up @@ -66,7 +66,7 @@ def test_http_insert_big_varchar(self):
})
for i in range(100):
self.insert(db_name, table_name, [
{"c1": "test_insert_big_varchar" * 1000}])
{"c1": "test_insert_big_varchar" * 1000}])
self.drop_table(db_name, table_name)
return

Expand Down Expand Up @@ -178,7 +178,7 @@ def test_http_insert_data_into_non_existent_table(self):
table_name = "test_insert_data_into_non_existent_table"
self.drop_table(db_name, table_name)
self.create_table(db_name, table_name, {
"c1": {"type": "integer", }, "c2": {"type": "integer", }})
"c1": {"type": "integer", }, "c2": {"type": "integer", }})
self.drop_table(db_name, table_name)

values = [{"c1": 1, "c2": 1}]
Expand Down Expand Up @@ -349,7 +349,6 @@ def test_http_batch_insert(self):
self.drop_table(db_name, table_name)
return

@pytest.mark.skip(reason="error")
@pytest.mark.parametrize("batch", [10, 1024])
@pytest.mark.parametrize("types", [(1, False), (1.1, False), ("1#$@!adf", False), ([1, 2, 3], True)])
def test_http_insert_with_invalid_data_type(self, batch, types):
Expand Down Expand Up @@ -411,7 +410,7 @@ def test_http_various_insert_types(self):

values = [{"c1": [1, 2, 3]} for _ in range(5)]
self.insert(db_name, table_name, values, {
"status_code": 500,
"status_code": 200,
})

self.drop_table(db_name, table_name)
Expand Down
2 changes: 2 additions & 0 deletions src/common/status.cppm
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,8 @@ export enum class ErrorCode : long {
kAggregateFunctionWithEmptyArgs = 3071,
kBlockNotExist = 3072,
kInvalidTopKType = 3073,
kInvalidCreateOption = 3074,
kInvalidDropOption = 3075,

// 4. Txn fail
kTxnRollback = 4001,
Expand Down
187 changes: 150 additions & 37 deletions src/network/http_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -101,17 +101,38 @@ class CreateDatabaseHandler final : public HttpRequestHandler {
String body_info = request->readBodyToString();
nlohmann::json body_info_json = nlohmann::json::parse(body_info);
String option = body_info_json["create_option"];
CreateDatabaseOptions create_option;

if (option == "ignore_if_exists") {
create_option.conflict_type_ = ConflictType::kIgnore;
HTTPStatus http_status;
nlohmann::json json_response;

CreateDatabaseOptions options;
if(body_info_json.contains("create_option")) {
auto create_option = body_info_json["create_option"];
if(create_option.is_string()) {
String option = create_option;
if(option == "ignore_if_exists") {
options.conflict_type_ = ConflictType::kIgnore;
} else if(option == "error") {
options.conflict_type_ = ConflictType::kError;
} else if(option == "replace_if_exists") {
options.conflict_type_ = ConflictType::kReplace;
} else {
json_response["error_code"] = 3074;
json_response["error_message"] = fmt::format("Invalid create option: {}", option);
http_status = HTTPStatus::CODE_500;
return ResponseFactory::createResponse(http_status, json_response.dump());
}
} else {
json_response["error_code"] = 3067;
json_response["error_message"] = "'CREATE OPTION' field value should be string type";
http_status = HTTPStatus::CODE_500;
return ResponseFactory::createResponse(http_status, json_response.dump());
}
}

// create database
auto result = infinity->CreateDatabase(database_name, create_option);
auto result = infinity->CreateDatabase(database_name, options);

HTTPStatus http_status;
nlohmann::json json_response;
if (result.IsOk()) {
json_response["error_code"] = 0;
http_status = HTTPStatus::CODE_200;
Expand All @@ -134,19 +155,37 @@ class DropDatabaseHandler final : public HttpRequestHandler {
String database_name = request->getPathVariable("database_name");

// get drop option
HTTPStatus http_status;
nlohmann::json json_response;

String body_info = request->readBodyToString();
nlohmann::json body_info_json = nlohmann::json::parse(body_info);
String option = body_info_json["drop_option"];
DropDatabaseOptions drop_option;

if (option == "ignore_if_not_exists") {
drop_option.conflict_type_ = ConflictType::kIgnore;
DropDatabaseOptions options;
if(body_info_json.contains("drop_option")) {
auto drop_option = body_info_json["drop_option"];
if(drop_option.is_string()) {
String option = drop_option;
if(option == "ignore_if_not_exists") {
options.conflict_type_ = ConflictType::kIgnore;
} else if(option == "error") {
options.conflict_type_ = ConflictType::kError;
} else {
json_response["error_code"] = 3075;
json_response["error_message"] = fmt::format("Invalid drop option: {}", option);
http_status = HTTPStatus::CODE_500;
return ResponseFactory::createResponse(http_status, json_response.dump());
}
} else {
json_response["error_code"] = 3067;
json_response["error_message"] = "'DROP OPTION' field value should be string type";
http_status = HTTPStatus::CODE_500;
return ResponseFactory::createResponse(http_status, json_response.dump());
}
}

auto result = infinity->DropDatabase(database_name, drop_option);
auto result = infinity->DropDatabase(database_name, options);

nlohmann::json json_response;
HTTPStatus http_status;
if (result.IsOk()) {
json_response["error_code"] = 0;
http_status = HTTPStatus::CODE_200;
Expand Down Expand Up @@ -278,15 +317,31 @@ class CreateTableHandler final : public HttpRequestHandler {
}
}
Vector<TableConstraint *> table_constraint;
String option = body_info_json["create_option"];
CreateTableOptions create_table_opts;
if (option == "ignore_if_exists") {
create_table_opts.conflict_type_ = ConflictType::kIgnore;
} else if (option == "replace_if_exists") {
create_table_opts.conflict_type_ = ConflictType::kReplace;

CreateTableOptions options;
if(body_info_json.contains("create_option")) {
auto create_option = body_info_json["create_option"];
if(create_option.is_string()) {
String option = create_option;
if(option == "ignore_if_exists") {
options.conflict_type_ = ConflictType::kIgnore;
} else if(option == "error") {
options.conflict_type_ = ConflictType::kError;
} else if(option == "replace_if_exists") {
options.conflict_type_ = ConflictType::kReplace;
} else {
json_response["error_code"] = 3074;
json_response["error_message"] = fmt::format("Invalid create option: {}", option);
http_status = HTTPStatus::CODE_500;
}
} else {
json_response["error_code"] = 3067;
json_response["error_message"] = "'CREATE OPTION' field value should be string type";
http_status = HTTPStatus::CODE_500;
}
}

auto result = infinity->CreateTable(database_name, table_name, column_definitions, table_constraint, create_table_opts);
auto result = infinity->CreateTable(database_name, table_name, column_definitions, table_constraint, options);

if (result.IsOk()) {
json_response["error_code"] = 0;
Expand All @@ -311,17 +366,34 @@ class DropTableHandler final : public HttpRequestHandler {
String body_info = request->readBodyToString();

nlohmann::json body_info_json = nlohmann::json::parse(body_info);
String option = body_info_json["drop_option"];
DropTableOptions drop_table_opts;
if (option == "ignore_if_not_exists") {
drop_table_opts.conflict_type_ = ConflictType::kIgnore;
}

auto result = infinity->DropTable(database_name, table_name, drop_table_opts);

HTTPStatus http_status;
http_status = HTTPStatus::CODE_200;
nlohmann::json json_response;

DropTableOptions options;
if(body_info_json.contains("drop_option")) {
auto drop_option = body_info_json["drop_option"];
if(drop_option.is_string()) {
String option = drop_option;
if(option == "ignore_if_not_exists") {
options.conflict_type_ = ConflictType::kIgnore;
} else if(option == "error") {
options.conflict_type_ = ConflictType::kError;
} else {
json_response["error_code"] = 3075;
json_response["error_message"] = fmt::format("Invalid drop option: {}", option);
http_status = HTTPStatus::CODE_500;
}
} else {
json_response["error_code"] = 3067;
json_response["error_message"] = "'DROP OPTION' field value should be string type";
http_status = HTTPStatus::CODE_500;
}
}

auto result = infinity->DropTable(database_name, table_name, options);

if (result.IsOk()) {
json_response["error_code"] = 0;
http_status = HTTPStatus::CODE_200;
Expand Down Expand Up @@ -872,11 +944,11 @@ class InsertHandler final : public HttpRequestHandler {

switch (value_type) {
case nlohmann::json::value_t::number_integer: {
const_expr->long_array_.emplace_back(value.template get<i64>());
const_expr->long_array_.emplace_back(value_ref.template get<i64>());
break;
}
case nlohmann::json::value_t::number_unsigned: {
const_expr->long_array_.emplace_back(value.template get<u64>());
const_expr->long_array_.emplace_back(value_ref.template get<u64>());
break;
}
default: {
Expand Down Expand Up @@ -1393,10 +1465,35 @@ class DropIndexHandler final : public HttpRequestHandler {
auto table_name = request->getPathVariable("table_name");
auto index_name = request->getPathVariable("index_name");

auto result = infinity->DropIndex(database_name, table_name, index_name, DropIndexOptions());
String body_info = request->readBodyToString();

nlohmann::json body_info_json = nlohmann::json::parse(body_info);

nlohmann::json json_response;
HTTPStatus http_status;
DropIndexOptions options;
if(body_info_json.contains("drop_option")) {
auto drop_option = body_info_json["drop_option"];
if(drop_option.is_string()) {
String option = drop_option;
if(option == "ignore_if_not_exists") {
options.conflict_type_ = ConflictType::kIgnore;
} else if(option == "error") {
options.conflict_type_ = ConflictType::kError;
} else {
json_response["error_code"] = 3074;
json_response["error_message"] = fmt::format("Invalid drop option: {}", option);
http_status = HTTPStatus::CODE_500;
}
} else {
json_response["error_code"] = 3067;
json_response["error_message"] = "'CREATE OPTION' field value should be string type";
http_status = HTTPStatus::CODE_500;
}
}

auto result = infinity->DropIndex(database_name, table_name, index_name, options);

if (result.IsOk()) {
json_response["error_code"] = 0;
http_status = HTTPStatus::CODE_200;
Expand All @@ -1422,19 +1519,35 @@ class CreateIndexHandler final : public HttpRequestHandler {
String body_info_str = request->readBodyToString();
nlohmann::json body_info_json = nlohmann::json::parse(body_info_str);

nlohmann::json json_response;
HTTPStatus http_status;

CreateIndexOptions options;
auto create_option = body_info_json["create_option"];
auto ignore_if_exists = create_option["ignore_if_exists"];
if (ignore_if_exists.is_boolean() && ignore_if_exists) {
options.conflict_type_ = ConflictType::kIgnore;
if(body_info_json.contains("create_option")) {
auto create_option = body_info_json["create_option"];
if(create_option.is_string()) {
String option = create_option;
if(option == "ignore_if_exists") {
options.conflict_type_ = ConflictType::kIgnore;
} else if(option == "error") {
options.conflict_type_ = ConflictType::kError;
} else if(option == "replace_if_exists") {
options.conflict_type_ = ConflictType::kReplace;
} else {
json_response["error_code"] = 3075;
json_response["error_message"] = fmt::format("Invalid create option: {}", option);
http_status = HTTPStatus::CODE_500;
}
} else {
json_response["error_code"] = 3067;
json_response["error_message"] = "'CREATE OPTION' field value should be string type";
http_status = HTTPStatus::CODE_500;
}
}

auto fields = body_info_json["fields"];
auto index = body_info_json["index"];

nlohmann::json json_response;
HTTPStatus http_status;

auto index_info_list = new Vector<IndexInfo *>();
{
auto index_info = new IndexInfo();
Expand Down
4 changes: 3 additions & 1 deletion src/storage/txn/txn.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -427,8 +427,10 @@ TxnTimeStamp Txn::Commit() {

if (txn_context_.GetTxnState() == TxnState::kToRollback) {
// abort because of conflict
RecoverableError(Status::TxnRollback(txn_id_));
LOG_ERROR(fmt::format("Txn: {} is rollbacked. rollback ts: {}", txn_id_, this->CommitTS()));
return this->CommitTS();
}

if (txn_context_.GetTxnState() != TxnState::kCommitted) {
UnrecoverableError("Transaction isn't in COMMITTED status.");
}
Expand Down

0 comments on commit 5b83f4d

Please sign in to comment.