Skip to content

Commit

Permalink
test: add binder tests for errors
Browse files Browse the repository at this point in the history
Add a test for every error not covered, as reported by lcov for lines
changed after the stringFormat change.
  • Loading branch information
Riolku committed Oct 17, 2023
1 parent d2f2783 commit 9a4d2a6
Show file tree
Hide file tree
Showing 21 changed files with 168 additions and 43 deletions.
2 changes: 2 additions & 0 deletions dataset/csv-error-tests/union-no-conversion.csv
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
"col:UNION(u UINT8, s INT8)"
a
4 changes: 3 additions & 1 deletion src/binder/bind/bind_create_macro.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#include "binder/bound_create_macro.h"
#include "common/exception/binder.h"
#include "common/string_format.h"
#include "common/string_utils.h"
#include "parser/create_macro.h"

using namespace kuzu::common;
Expand All @@ -13,6 +14,7 @@ namespace binder {
std::unique_ptr<BoundStatement> Binder::bindCreateMacro(const Statement& statement) {
auto& createMacro = reinterpret_cast<const CreateMacro&>(statement);
auto macroName = createMacro.getMacroName();
StringUtils::toUpper(macroName);
if (catalog.getReadOnlyVersion()->containMacro(macroName)) {
throw BinderException{stringFormat("Macro {} already exists.", macroName)};
}
Expand All @@ -23,7 +25,7 @@ std::unique_ptr<BoundStatement> Binder::bindCreateMacro(const Statement& stateme
auto scalarMacro =
std::make_unique<function::ScalarMacroFunction>(createMacro.getMacroExpression()->copy(),
createMacro.getPositionalArgs(), std::move(defaultArgs));
return std::make_unique<BoundCreateMacro>(macroName, std::move(scalarMacro));
return std::make_unique<BoundCreateMacro>(std::move(macroName), std::move(scalarMacro));
}

} // namespace binder
Expand Down
3 changes: 0 additions & 3 deletions src/binder/bind/bind_file_scan.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ std::vector<std::string> Binder::bindFilePaths(const std::vector<std::string>& f
boundFilePaths.insert(
boundFilePaths.end(), globbedFilePaths.begin(), globbedFilePaths.end());
}
if (boundFilePaths.empty()) {
throw BinderException{stringFormat("Invalid file path: {}.", filePaths[0])};
}
return boundFilePaths;
}

Expand Down
7 changes: 4 additions & 3 deletions src/binder/bind/bind_graph_pattern.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -186,9 +186,10 @@ static std::unique_ptr<Expression> createPropertyExpression(const std::string& p
}
for (auto type : propertyDataTypes) {
if (*propertyDataTypes[0] != *type) {
stringFormat("Expected the same data type for property {} but found {} and {}.",
propertyName, LogicalTypeUtils::dataTypeToString(*type),
LogicalTypeUtils::dataTypeToString(*propertyDataTypes[0]));
throw BinderException(
stringFormat("Expected the same data type for property {} but found {} and {}.",
propertyName, LogicalTypeUtils::dataTypeToString(*type),
LogicalTypeUtils::dataTypeToString(*propertyDataTypes[0])));
}
}
return make_unique<PropertyExpression>(*propertyDataTypes[0], propertyName, uniqueVariableName,
Expand Down
2 changes: 2 additions & 0 deletions src/catalog/catalog_content.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -230,10 +230,12 @@ std::unique_ptr<CatalogContent> CatalogContent::copy() const {
void CatalogContent::validateStorageVersion(storage_version_t savedStorageVersion) {
auto storageVersion = StorageVersionInfo::getStorageVersion();
if (savedStorageVersion != storageVersion) {
// LCOV_EXCL_START
throw RuntimeException(
stringFormat("Trying to read a database file with a different version. "
"Database file version: {}, Current build storage version: {}",
savedStorageVersion, storageVersion));
// LCOV_EXCL_END
}
}

Expand Down
18 changes: 8 additions & 10 deletions src/catalog/table_schema.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,16 @@ std::vector<Property*> TableSchema::getProperties() const {
return propertiesToReturn;
}

std::string TableSchema::getPropertyName(property_id_t propertyID) const {
for (auto& property : properties) {
if (property->getPropertyID() == propertyID) {
return property->getName();
}
}
throw RuntimeException(stringFormat(
"Table: {} doesn't have a property with propertyID={}.", tableName, propertyID));
}

property_id_t TableSchema::getPropertyID(const std::string& propertyName) const {
for (auto& property : properties) {
if (property->getName() == propertyName) {
return property->getPropertyID();
}
}
// LCOV_EXCL_START
throw RuntimeException(stringFormat(
"Table: {} doesn't have a property with propertyName={}.", tableName, propertyName));
// LCOV_EXCL_END
}

// TODO(Guodong): Instead of looping over properties, cache a map between propertyID and columnID.
Expand All @@ -66,8 +58,10 @@ Property* TableSchema::getProperty(property_id_t propertyID) const {
return property.get();
}
}
// LCOV_EXCL_START
throw RuntimeException(stringFormat(
"Table: {} doesn't have a property with propertyID={}.", tableName, propertyID));
// LCOV_EXCL_END
}

void TableSchema::renameProperty(property_id_t propertyID, const std::string& newName) {
Expand All @@ -77,7 +71,9 @@ void TableSchema::renameProperty(property_id_t propertyID, const std::string& ne
return;
}
}
// LCOV_EXCL_START
throw InternalException(stringFormat("Property with id={} not found.", propertyID));
// LCOV_EXCL_END
}

void TableSchema::serialize(FileInfo* fileInfo, uint64_t& offset) {
Expand Down Expand Up @@ -118,7 +114,9 @@ std::unique_ptr<TableSchema> TableSchema::deserialize(FileInfo* fileInfo, uint64
result = RdfGraphSchema::deserialize(fileInfo, offset);
} break;
default: {
// LCOV_EXCL_START
throw NotImplementedException{"TableSchema::deserialize"};
// LCOV_EXCL_END
}
}
result->tableName = tableName;
Expand Down
9 changes: 5 additions & 4 deletions src/common/assert.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,12 @@ namespace kuzu {
namespace common {

void kuAssertInternal(bool condition, const char* condition_name, const char* file, int linenr) {
if (condition) {
return;
if (!condition) {
// LCOV_EXCL_START
throw InternalException(stringFormat(
"Assertion triggered in file \"{}\" on line {}: {}", file, linenr, condition_name));
// LCOV_EXCL_END
}
throw InternalException(stringFormat(
"Assertion triggered in file \"{}\" on line {}: {}", file, linenr, condition_name));
}

} // namespace common
Expand Down
46 changes: 34 additions & 12 deletions src/common/file_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

#include "common/exception/storage.h"
#include "common/string_format.h"
#include "common/system_message.h"
#include "glob/glob.hpp"

#ifdef _WIN32
Expand Down Expand Up @@ -32,13 +33,14 @@ int64_t FileInfo::getFileSize() {
if (!GetFileSizeEx((HANDLE)handle, &size)) {
auto error = GetLastError();
throw StorageException(stringFormat("Cannot read size of file. path: {} - Error {}: {}",
path, error, std::system_category().message(error)));
path, error, systemErrMessage(error)));
}
return size.QuadPart;
#else
struct stat s;
if (fstat(fd, &s) == -1) {
return -1;
throw StorageException(stringFormat(
"Cannot read size of file. path: {} - Error {}: {}", path, errno, posixErrMessage()));

Check warning on line 43 in src/common/file_utils.cpp

View check run for this annotation

Codecov / codecov/patch

src/common/file_utils.cpp#L42-L43

Added lines #L42 - L43 were not covered by tests
}
return s.st_size;
#endif
Expand Down Expand Up @@ -99,10 +101,6 @@ std::unique_ptr<FileInfo> FileUtils::openFile(

void FileUtils::writeToFile(
FileInfo* fileInfo, const uint8_t* buffer, uint64_t numBytes, uint64_t offset) {
auto fileSize = fileInfo->getFileSize();
if (fileSize == -1) {
throw Exception(stringFormat("File {} not open.", fileInfo->path));
}
uint64_t remainingNumBytesToWrite = numBytes;
uint64_t bufferOffset = 0;
// Split large writes to 1GB at a time
Expand All @@ -128,10 +126,13 @@ void FileUtils::writeToFile(
uint64_t numBytesWritten =
pwrite(fileInfo->fd, buffer + bufferOffset, numBytesToWrite, offset);
if (numBytesWritten != numBytesToWrite) {
// LCOV_EXCL_START
throw Exception(
stringFormat("Cannot write to file. path: {} fileDescriptor: {} offsetToWrite: {} "
"numBytesToWrite: {} numBytesWritten: {}.",
fileInfo->path, fileInfo->fd, offset, numBytesToWrite, numBytesWritten));
"numBytesToWrite: {} numBytesWritten: {}. Error: {}",
fileInfo->path, fileInfo->fd, offset, numBytesToWrite, numBytesWritten,
posixErrMessage()));
// LCOV_EXCL_END
}
#endif
remainingNumBytesToWrite -= numBytesWritten;
Expand All @@ -146,8 +147,10 @@ void FileUtils::copyFile(
return;
std::error_code errorCode;
if (!std::filesystem::copy_file(from, to, options, errorCode)) {
// LCOV_EXCL_START
throw Exception(stringFormat(
"Error copying file {} to {}. ErrorMessage: {}", from, to, errorCode.message()));
// LCOV_EXCL_END
}
}

Expand Down Expand Up @@ -180,24 +183,32 @@ void FileUtils::readFromFile(
#else
auto numBytesRead = pread(fileInfo->fd, buffer, numBytes, position);
if (numBytesRead != numBytes && fileInfo->getFileSize() != position + numBytesRead) {
// LCOV_EXCL_START
throw Exception(stringFormat("Cannot read from file: {} fileDescriptor: {} "
"numBytesRead: {} numBytesToRead: {} position: {}",
fileInfo->path, fileInfo->fd, numBytesRead, numBytes, position));
// LCOV_EXCL_END
}
#endif
}

void FileUtils::createDir(const std::string& dir) {
try {
if (std::filesystem::exists(dir)) {
// LCOV_EXCL_START
throw Exception(stringFormat("Directory {} already exists.", dir));
// LCOV_EXCL_END
}
if (!std::filesystem::create_directory(dir)) {
// LCOV_EXCL_START
throw Exception(stringFormat(
"Directory {} cannot be created. Check if it exists and remove it.", dir));
// LCOV_EXCL_END
}
} catch (std::exception& e) {
// LCOV_EXCL_START
throw Exception(stringFormat("Failed to create directory {} due to: {}", dir, e.what()));
// LCOV_EXCL_END
}
}

Expand All @@ -212,8 +223,10 @@ void FileUtils::removeDir(const std::string& dir) {
if (!fileOrPathExists(dir))
return;
if (!std::filesystem::remove_all(dir, removeErrorCode)) {
// LCOV_EXCL_START
throw Exception(stringFormat(
"Error removing directory {}. Error Message: {}", dir, removeErrorCode.message()));
"Error removing directory {}. Error Message: {}", dir, removeErrorCode.message()));
// LCOV_EXCL_END
}
}

Expand All @@ -224,17 +237,21 @@ void FileUtils::renameFileIfExists(const std::string& oldName, const std::string
std::error_code errorCode;
std::filesystem::rename(oldName, newName, errorCode);
if (errorCode.value() != 0) {
// LCOV_EXCL_START
throw Exception(stringFormat("Error replacing file {} to {}. ErrorMessage: {}", oldName,
newName, errorCode.message()));
// LCOV_EXCL_END
}
}

void FileUtils::removeFileIfExists(const std::string& path) {
if (!fileOrPathExists(path))
return;
if (remove(path.c_str()) != 0) {
throw Exception(
stringFormat("Error removing directory or file {}. Error Message: ", path));
// LCOV_EXCL_START
throw Exception(stringFormat(
"Error removing directory or file {}. Error Message: {}", path, posixErrMessage()));
// LCOV_EXCL_END
}
}

Expand Down Expand Up @@ -268,7 +285,12 @@ void FileUtils::truncateFileToSize(FileInfo* fileInfo, uint64_t size) {
std::system_category().message(error)));
}
#else
ftruncate(fileInfo->fd, size);
if (ftruncate(fileInfo->fd, size) < 0) {
// LCOV_EXCL_START
throw Exception(
stringFormat("Failed to truncate file {}: {}", fileInfo->path, posixErrMessage()));
// LCOV_EXCL_END
}
#endif
}

Expand Down
9 changes: 5 additions & 4 deletions src/common/types/blob.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,10 @@ uint64_t Blob::fromString(const char* str, uint64_t length, uint8_t* resultBuffe
} else if (str[i] <= 127) {
resultBuffer[resultPos++] = str[i];
} else {
throw ConversionException(
"Invalid byte encountered in STRING -> BLOB conversion. All non-ascii characters "
"must be escaped with hex codes (e.g. \\xAA)");
// LCOV_EXCL_START
throw InternalException("Invalid byte encountered in STRING -> BLOB conversion that "
"should have been caught during getBlobSize");
// LCOV_EXCL_END
}
}
return resultPos;
Expand Down Expand Up @@ -89,7 +90,7 @@ void Blob::validateHexCode(const uint8_t* blobStr, uint64_t length, uint64_t cur
if (curPos + HexFormatConstants::LENGTH > length) {
throw ConversionException(
"Invalid hex escape code encountered in string -> blob conversion: "
"unterminated escape code at end of blob");
"unterminated escape code at end of string");
}
if (memcmp(blobStr + curPos, HexFormatConstants::PREFIX, HexFormatConstants::PREFIX_LENGTH) !=
0 ||
Expand Down
2 changes: 2 additions & 0 deletions src/function/base_lower_upper_operation.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,12 @@ uint32_t BaseLowerUpperFunction::getResultLen(char* inputStr, uint32_t inputLen,
int size = 0;
int codepoint = utf8proc_codepoint(inputStr + i, size);
if (codepoint < 0) {
// LCOV_EXCL_START
// TODO(Xiyang): We shouldn't allow invalid UTF-8 to enter a string column.
std::string funcName = isUpper ? "UPPER" : "LOWER";
throw RuntimeException(
common::stringFormat("Failed calling {}: Invalid UTF-8.", funcName));
// LCOV_EXCL_END
}
int convertedCodepoint =
isUpper ? utf8proc_toupper(codepoint) : utf8proc_tolower(codepoint);
Expand Down
2 changes: 1 addition & 1 deletion src/function/vector_path_functions.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ std::unique_ptr<FunctionBindData> PropertiesVectorFunction::bindFunc(
const binder::expression_vector& arguments, FunctionDefinition* definition) {
if (arguments[1]->expressionType != ExpressionType::LITERAL) {
throw BinderException(stringFormat(
"Expect literal input as the second argument for {}().", PROPERTIES_FUNC_NAME));
"Expected literal input as the second argument for {}().", PROPERTIES_FUNC_NAME));
}
auto key = ((binder::LiteralExpression&)*arguments[1]).getValue()->getValue<std::string>();
auto listType = arguments[0]->getDataType();
Expand Down
2 changes: 0 additions & 2 deletions src/include/catalog/table_schema.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,6 @@ class TableSchema {
std::move(propertyName), std::move(dataType), increaseNextPropertyID(), tableID));
}

std::string getPropertyName(common::property_id_t propertyID) const;

common::property_id_t getPropertyID(const std::string& propertyName) const;
common::column_id_t getColumnID(common::property_id_t propertyID) const;

Expand Down
4 changes: 4 additions & 0 deletions src/processor/map/expression_mapper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -68,8 +68,10 @@ std::unique_ptr<ExpressionEvaluator> ExpressionMapper::getEvaluator(
} else if (canEvaluateAsFunction(expressionType)) {
return getFunctionEvaluator(expression, schema);
} else {
// LCOV_EXCL_START
throw NotImplementedException(stringFormat(
"Cannot evaluate expression with type {}.", expressionTypeToString(expressionType)));
// LCOV_EXCL_END
}
}

Expand All @@ -84,8 +86,10 @@ std::unique_ptr<ExpressionEvaluator> ExpressionMapper::getConstantEvaluator(
} else if (canEvaluateAsFunction(expressionType)) {
return getFunctionEvaluator(expression, nullptr);
} else {
// LCOV_EXCL_START
throw NotImplementedException(stringFormat(
"Cannot evaluate expression with type {}.", expressionTypeToString(expressionType)));
// LCOV_EXCL_END
}
}

Expand Down
3 changes: 2 additions & 1 deletion src/processor/operator/persistent/reader/csv/driver.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -586,7 +586,8 @@ void copyStringToVector(ValueVector* vector, uint64_t rowToAdd, std::string_view
}
}
if (selectedFieldIdx == INVALID_STRUCT_FIELD_IDX) {
throw ParserException{stringFormat("No parsing rule matches value: {}.", strVal)};
throw ConversionException{stringFormat("Could not convert to union type {}: {}.",
LogicalTypeUtils::dataTypeToString(type), strVal)};
}
StructVector::getFieldVector(vector, UnionType::TAG_FIELD_IDX)
->setValue(rowToAdd, selectedFieldIdx);
Expand Down
2 changes: 1 addition & 1 deletion src/storage/in_mem_storage_structure/in_mem_lists.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void InMemLists::copyArrowArray(arrow::Array* boundNodeOffsets, arrow::Array* po
} break;
// TODO(Ziyi): Add support of VAR_LIST and more native parquet data types.
default: {
throw CopyException("Unsupported data type ");
throw CopyException("Unsupported data type");

Check warning on line 91 in src/storage/in_mem_storage_structure/in_mem_lists.cpp

View check run for this annotation

Codecov / codecov/patch

src/storage/in_mem_storage_structure/in_mem_lists.cpp#L91

Added line #L91 was not covered by tests
}
}
}
Expand Down
2 changes: 2 additions & 0 deletions src/storage/storage_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ namespace storage {
storage_version_t StorageVersionInfo::getStorageVersion() {
auto storageVersionInfo = getStorageVersionInfo();
if (!storageVersionInfo.contains(KUZU_STORAGE_VERSION)) {
// LCOV_EXCL_START
throw RuntimeException(
stringFormat("Invalid storage version name: {}", KUZU_STORAGE_VERSION));
// LCOV_EXCL_END
}
return storageVersionInfo.at(KUZU_STORAGE_VERSION);
}
Expand Down
Loading

0 comments on commit 9a4d2a6

Please sign in to comment.