Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix #44, #47, #53 #69

Merged
merged 11 commits into from
Oct 19, 2023
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
3 changes: 1 addition & 2 deletions .github/workflows/Windows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -60,8 +60,7 @@ jobs:
env:
SQLITE_TPCH_GENERATED: 1
run: |
export EXT_PATH=`pwd`/test/sql
build/release/test/Release/unittest.exe `$EXT_PATH*`
build/release/test/Release/unittest.exe --test-dir . "test/*"

- uses: actions/upload-artifact@v2
with:
Expand Down
8 changes: 4 additions & 4 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ set(PARAMETERS "-warnings")
build_loadable_extension(${TARGET_NAME} ${PARAMETERS} ${EXTENSION_OBJECT_FILES})

install(
TARGETS ${EXTENSION_NAME}
EXPORT "${DUCKDB_EXPORT_SET}"
LIBRARY DESTINATION "${INSTALL_LIB_DIR}"
ARCHIVE DESTINATION "${INSTALL_LIB_DIR}")
TARGETS ${EXTENSION_NAME}
EXPORT "${DUCKDB_EXPORT_SET}"
LIBRARY DESTINATION "${INSTALL_LIB_DIR}"
ARCHIVE DESTINATION "${INSTALL_LIB_DIR}")
2 changes: 2 additions & 0 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,10 @@ test_debug: debug
./build/release/test/unittest "$(PROJ_DIR)test/*"

format:
cp duckdb/.clang-format .
find src/ -iname *.hpp -o -iname *.cpp | xargs clang-format --sort-includes=0 -style=file -i
cmake-format -i CMakeLists.txt
rm .clang-format

update:
git submodule update --remote --merge
Binary file added data/db/boolean.db
Binary file not shown.
Binary file added data/db/issue44.db
Binary file not shown.
Binary file added data/db/issue47.db
Binary file not shown.
16 changes: 8 additions & 8 deletions data/sql/tpch-export.duckdb
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
CALL dbgen(sf=0.1);
COPY nation TO 'nation.tbl';
COPY region TO 'region.tbl';
COPY part TO 'part.tbl';
COPY supplier TO 'supplier.tbl';
COPY partsupp TO 'partsupp.tbl';
COPY customer TO 'customer.tbl';
COPY orders TO 'orders.tbl';
COPY lineitem TO 'lineitem.tbl';
COPY nation TO 'nation.tbl' (HEADER FALSE);
COPY region TO 'region.tbl' (HEADER FALSE);
COPY part TO 'part.tbl' (HEADER FALSE);
COPY supplier TO 'supplier.tbl' (HEADER FALSE);
COPY partsupp TO 'partsupp.tbl' (HEADER FALSE);
COPY customer TO 'customer.tbl' (HEADER FALSE);
COPY orders TO 'orders.tbl' (HEADER FALSE);
COPY lineitem TO 'lineitem.tbl' (HEADER FALSE);
2 changes: 1 addition & 1 deletion duckdb
Submodule duckdb updated 659 files
3 changes: 2 additions & 1 deletion src/include/sqlite_db.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ class SQLiteDB {
void GetViewInfo(const string &view_name, string &sql);
void GetIndexInfo(const string &index_name, string &sql, string &table_name);
idx_t RunPragma(string pragma_name);
//! Gets the max row id of a table, returns false if the table does not have a rowid column
//! Gets the max row id of a table, returns false if the table does not have a
//! rowid column
bool GetMaxRowId(const string &table_name, idx_t &row_id);
bool ColumnExists(const string &table_name, const string &column_name);
vector<IndexInfo> GetIndexInfo(const string &table_name);
Expand Down
8 changes: 4 additions & 4 deletions src/include/sqlite_scanner_extension.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,10 @@ using namespace duckdb;

class SqliteScannerExtension : public Extension {
public:
std::string Name() override {
return "sqlite_scanner";
}
void Load(DuckDB &db) override;
std::string Name() override {
return "sqlite_scanner";
}
void Load(DuckDB &db) override;
};

extern "C" {
Expand Down
4 changes: 3 additions & 1 deletion src/include/sqlite_stmt.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <cstddef>

namespace duckdb {
struct SqliteBindData;

class SQLiteStatement {
public:
Expand Down Expand Up @@ -44,7 +45,8 @@ class SQLiteStatement {
int GetType(idx_t col);
bool IsOpen();
void Close();
void CheckTypeMatches(sqlite3_value *val, int sqlite_column_type, int expected_type, idx_t col_idx);
void CheckTypeMatches(const SqliteBindData &bind_data, sqlite3_value *val, int sqlite_column_type,
int expected_type, idx_t col_idx);
void CheckTypeIsFloatOrInteger(sqlite3_value *val, int sqlite_column_type, idx_t col_idx);
void Reset();
};
Expand Down
3 changes: 2 additions & 1 deletion src/include/storage/sqlite_delete.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ namespace duckdb {

class SQLiteDelete : public PhysicalOperator {
public:
SQLiteDelete(LogicalOperator &op, TableCatalogEntry &table);
SQLiteDelete(LogicalOperator &op, TableCatalogEntry &table, idx_t row_id_index);

//! The table to delete from
TableCatalogEntry &table;
idx_t row_id_index;

public:
// Source interface
Expand Down
9 changes: 5 additions & 4 deletions src/include/storage/sqlite_table_entry.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@ namespace duckdb {

class SQLiteTableEntry : public TableCatalogEntry {
public:
SQLiteTableEntry(Catalog &catalog, SchemaCatalogEntry &schema, CreateTableInfo &info);
SQLiteTableEntry(Catalog &catalog, SchemaCatalogEntry &schema, CreateTableInfo &info, bool all_varchar);

bool all_varchar;

public:
unique_ptr<BaseStatistics> GetStatistics(ClientContext &context, column_t column_id) override;
Expand All @@ -23,9 +25,8 @@ class SQLiteTableEntry : public TableCatalogEntry {

TableStorageInfo GetStorageInfo(ClientContext &context) override;

void BindUpdateConstraints(LogicalGet &get, LogicalProjection &proj, LogicalUpdate &update,
ClientContext &context) override;

void BindUpdateConstraints(LogicalGet &get, LogicalProjection &proj, LogicalUpdate &update,
ClientContext &context) override;
};

} // namespace duckdb
8 changes: 5 additions & 3 deletions src/sqlite_db.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,8 @@ SQLiteDB SQLiteDB::Open(const string &path, bool is_read_only, bool is_shared) {
flags |= SQLITE_OPEN_READWRITE | SQLITE_OPEN_CREATE;
}
if (!is_shared) {
// FIXME: we should just make sure we are not re-using the same `sqlite3` object across threads
// FIXME: we should just make sure we are not re-using the same `sqlite3`
// object across threads
flags |= SQLITE_OPEN_NOMUTEX;
}
flags |= SQLITE_OPEN_EXRESCODE;
Expand Down Expand Up @@ -166,7 +167,7 @@ void SQLiteDB::GetTableInfo(const string &table_name, ColumnList &columns, vecto
auto column_type = all_varchar ? LogicalType::VARCHAR : SQLiteUtils::TypeToLogicalType(sqlite_type);

ColumnDefinition column(std::move(sqlite_colname), std::move(column_type));
if (!default_value.empty()) {
if (!default_value.empty() && default_value != "\"\"") {
auto expressions = Parser::ParseExpressionList(default_value);
if (expressions.empty()) {
throw InternalException("Expression list is empty");
Expand Down Expand Up @@ -246,7 +247,8 @@ vector<IndexInfo> SQLiteDB::GetIndexInfo(const string &table_name) {
}

// now query the set of unique constraints for the table
stmt = Prepare(StringUtil::Format("SELECT name FROM pragma_index_list('%s') WHERE \"unique\" AND origin='u'",
stmt = Prepare(StringUtil::Format("SELECT name FROM pragma_index_list('%s') "
"WHERE \"unique\" AND origin='u'",
SQLiteUtils::SanitizeString(table_name)));
vector<string> unique_indexes;
while (stmt.Step()) {
Expand Down
4 changes: 2 additions & 2 deletions src/sqlite_extension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ static void LoadInternal(DatabaseInstance &db) {
}

void SqliteScannerExtension::Load(DuckDB &db) {
LoadInternal(*db.instance);
LoadInternal(*db.instance);
}

DUCKDB_EXTENSION_API void sqlite_scanner_init(duckdb::DatabaseInstance &db) {
LoadInternal(db);
LoadInternal(db);
}

DUCKDB_EXTENSION_API const char *sqlite_scanner_version() {
Expand Down
16 changes: 8 additions & 8 deletions src/sqlite_scanner.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ static void SqliteInitInternal(ClientContext &context, const SqliteBindData &bin
D_ASSERT(rowid_min <= rowid_max);

local_state.done = false;
// we may have leftover statements or connections from a previous call to this function
// we may have leftover statements or connections from a previous call to this
// function
local_state.stmt.Close();
if (!local_state.db) {
local_state.owned_db = SQLiteDB::Open(bind_data.file_name.c_str());
Expand All @@ -102,7 +103,8 @@ static void SqliteInitInternal(ClientContext &context, const SqliteBindData &bin
auto sql =
StringUtil::Format("SELECT %s FROM \"%s\"", col_names, SQLiteUtils::SanitizeIdentifier(bind_data.table_name));
if (bind_data.rows_per_group != idx_t(-1)) {
// we are scanning a subset of the rows - generate a WHERE clause based on the rowid
// we are scanning a subset of the rows - generate a WHERE clause based on
// the rowid
auto where_clause = StringUtil::Format(" WHERE ROWID BETWEEN %d AND %d", rowid_min, rowid_max);
sql += where_clause;
} else {
Expand Down Expand Up @@ -197,27 +199,25 @@ static void SqliteScan(ClientContext &context, TableFunctionInput &data, DataChu
auto val = stmt.GetValue<sqlite3_value *>(col_idx);
switch (out_vec.GetType().id()) {
case LogicalTypeId::BIGINT:
stmt.CheckTypeMatches(val, sqlite_column_type, SQLITE_INTEGER, col_idx);
stmt.CheckTypeMatches(bind_data, val, sqlite_column_type, SQLITE_INTEGER, col_idx);
FlatVector::GetData<int64_t>(out_vec)[out_idx] = sqlite3_value_int64(val);
break;
case LogicalTypeId::DOUBLE:
stmt.CheckTypeIsFloatOrInteger(val, sqlite_column_type, col_idx);
FlatVector::GetData<double>(out_vec)[out_idx] = sqlite3_value_double(val);
break;
case LogicalTypeId::VARCHAR:
if (!bind_data.all_varchar) {
stmt.CheckTypeMatches(val, sqlite_column_type, SQLITE_TEXT, col_idx);
}
stmt.CheckTypeMatches(bind_data, val, sqlite_column_type, SQLITE_TEXT, col_idx);
FlatVector::GetData<string_t>(out_vec)[out_idx] = StringVector::AddString(
out_vec, (const char *)sqlite3_value_text(val), sqlite3_value_bytes(val));
break;
case LogicalTypeId::DATE:
stmt.CheckTypeMatches(val, sqlite_column_type, SQLITE_TEXT, col_idx);
stmt.CheckTypeMatches(bind_data, val, sqlite_column_type, SQLITE_TEXT, col_idx);
FlatVector::GetData<date_t>(out_vec)[out_idx] =
Date::FromCString((const char *)sqlite3_value_text(val), sqlite3_value_bytes(val));
break;
case LogicalTypeId::TIMESTAMP:
stmt.CheckTypeMatches(val, sqlite_column_type, SQLITE_TEXT, col_idx);
stmt.CheckTypeMatches(bind_data, val, sqlite_column_type, SQLITE_TEXT, col_idx);
FlatVector::GetData<timestamp_t>(out_vec)[out_idx] =
Timestamp::FromCString((const char *)sqlite3_value_text(val), sqlite3_value_bytes(val));
break;
Expand Down
8 changes: 7 additions & 1 deletion src/sqlite_stmt.cpp
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
#include "sqlite_stmt.hpp"
#include "sqlite_db.hpp"
#include "sqlite_scanner.hpp"

namespace duckdb {

Expand Down Expand Up @@ -55,8 +56,13 @@ void SQLiteStatement::Close() {
stmt = nullptr;
}

void SQLiteStatement::CheckTypeMatches(sqlite3_value *val, int sqlite_column_type, int expected_type, idx_t col_idx) {
void SQLiteStatement::CheckTypeMatches(const SqliteBindData &bind_data, sqlite3_value *val, int sqlite_column_type,
int expected_type, idx_t col_idx) {
D_ASSERT(stmt);
if (bind_data.all_varchar) {
// no type check required
return;
}
if (sqlite_column_type != expected_type) {
auto column_name = string(sqlite3_column_name(stmt, int(col_idx)));
auto value_as_text = string((char *)sqlite3_value_text(val));
Expand Down
29 changes: 19 additions & 10 deletions src/sqlite_utils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,37 +55,46 @@ LogicalType SQLiteUtils::ToSQLiteType(const LogicalType &input) {
}

LogicalType SQLiteUtils::TypeToLogicalType(const string &sqlite_type) {
// type affinity rules are taken from here: https://www.sqlite.org/datatype3.html
// type affinity rules are taken from here:
// https://www.sqlite.org/datatype3.html

// If the declared type contains the string "INT" then it is assigned INTEGER affinity.
// If the declared type contains the string "INT" then it is assigned INTEGER
// affinity.
if (StringUtil::Contains(sqlite_type, "int")) {
return LogicalType::BIGINT;
}
// If the declared type of the column contains any of the strings "CHAR", "CLOB", or "TEXT" then that column has
// TEXT affinity. Notice that the type VARCHAR contains the string "CHAR" and is thus assigned TEXT affinity.

// boolean
if (StringUtil::Contains(sqlite_type, "bool")) {
return LogicalType::BIGINT;
}

// If the declared type of the column contains any of the strings "CHAR",
// "CLOB", or "TEXT" then that column has TEXT affinity. Notice that the type
// VARCHAR contains the string "CHAR" and is thus assigned TEXT affinity.
if (StringUtil::Contains(sqlite_type, "char") || StringUtil::Contains(sqlite_type, "clob") ||
StringUtil::Contains(sqlite_type, "text")) {
return LogicalType::VARCHAR;
}

// If the declared type for a column contains the string "BLOB" or if no type is specified then the column has
// affinity BLOB.
// If the declared type for a column contains the string "BLOB" or if no type
// is specified then the column has affinity BLOB.
if (StringUtil::Contains(sqlite_type, "blob") || sqlite_type.empty()) {
return LogicalType::BLOB;
}

// If the declared type for a column contains any of the strings "REAL", "FLOA", or "DOUB" then the column has REAL
// affinity.
// If the declared type for a column contains any of the strings "REAL",
// "FLOA", or "DOUB" then the column has REAL affinity.
if (StringUtil::Contains(sqlite_type, "real") || StringUtil::Contains(sqlite_type, "floa") ||
StringUtil::Contains(sqlite_type, "doub")) {
return LogicalType::DOUBLE;
}
// Otherwise, the affinity is NUMERIC.
// now numeric sounds simple, but it is rather complex:
// A column with NUMERIC affinity may contain values using all five storage classes.
// A column with NUMERIC affinity may contain values using all five storage
// classes.
// ...
// we add some more extra rules to try to be somewhat sane

if (sqlite_type == "date") {
return LogicalType::DATE;
}
Expand Down
7 changes: 4 additions & 3 deletions src/storage/sqlite_catalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ SQLiteDB *SQLiteCatalog::GetInMemoryDatabase() {
}
lock_guard<mutex> l(in_memory_lock);
if (active_in_memory) {
throw TransactionException("Only a single transaction can be active on an in-memory SQLite database at a time");
throw TransactionException("Only a single transaction can be active on an "
"in-memory SQLite database at a time");
}
active_in_memory = true;
return &in_memory_db;
Expand All @@ -66,8 +67,8 @@ void SQLiteCatalog::ReleaseInMemoryDatabase() {
}
lock_guard<mutex> l(in_memory_lock);
if (!active_in_memory) {
throw InternalException(
"ReleaseInMemoryDatabase called but there is no active transaction on an in-memory database");
throw InternalException("ReleaseInMemoryDatabase called but there is no "
"active transaction on an in-memory database");
}
active_in_memory = false;
}
Expand Down
10 changes: 6 additions & 4 deletions src/storage/sqlite_delete.cpp
Original file line number Diff line number Diff line change
@@ -1,15 +1,16 @@
#include "storage/sqlite_delete.hpp"
#include "storage/sqlite_table_entry.hpp"
#include "duckdb/planner/operator/logical_delete.hpp"
#include "duckdb/planner/expression/bound_reference_expression.hpp"
#include "storage/sqlite_catalog.hpp"
#include "storage/sqlite_transaction.hpp"
#include "sqlite_db.hpp"
#include "sqlite_stmt.hpp"

namespace duckdb {

SQLiteDelete::SQLiteDelete(LogicalOperator &op, TableCatalogEntry &table)
: PhysicalOperator(PhysicalOperatorType::EXTENSION, op.types, 1), table(table) {
SQLiteDelete::SQLiteDelete(LogicalOperator &op, TableCatalogEntry &table, idx_t row_id_index)
: PhysicalOperator(PhysicalOperatorType::EXTENSION, op.types, 1), table(table), row_id_index(row_id_index) {
}

//===--------------------------------------------------------------------===//
Expand Down Expand Up @@ -48,7 +49,7 @@ SinkResultType SQLiteDelete::Sink(ExecutionContext &context, DataChunk &chunk, O
auto &gstate = input.global_state.Cast<SQLiteDeleteGlobalState>();

chunk.Flatten();
auto &row_identifiers = chunk.data[0];
auto &row_identifiers = chunk.data[row_id_index];
auto row_data = FlatVector::GetData<row_t>(row_identifiers);
for (idx_t i = 0; i < chunk.size(); i++) {
gstate.statement.Bind<int64_t>(0, row_data[i]);
Expand Down Expand Up @@ -89,7 +90,8 @@ unique_ptr<PhysicalOperator> SQLiteCatalog::PlanDelete(ClientContext &context, L
if (op.return_chunk) {
throw BinderException("RETURNING clause not yet supported for deletion of a SQLite table");
}
auto insert = make_uniq<SQLiteDelete>(op, op.table);
auto &bound_ref = op.expressions[0]->Cast<BoundReferenceExpression>();
auto insert = make_uniq<SQLiteDelete>(op, op.table, bound_ref.index);
insert->children.push_back(std::move(plan));
return std::move(insert);
}
Expand Down
6 changes: 4 additions & 2 deletions src/storage/sqlite_schema_entry.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,8 @@ string GetCreateViewSQL(CreateViewInfo &info) {

optional_ptr<CatalogEntry> SQLiteSchemaEntry::CreateView(CatalogTransaction transaction, CreateViewInfo &info) {
if (info.sql.empty()) {
throw BinderException("Cannot create view in SQLite that originated from an empty SQL statement");
throw BinderException("Cannot create view in SQLite that originated from "
"an empty SQL statement");
}
if (info.on_conflict == OnCreateConflict::REPLACE_ON_CONFLICT) {
// CREATE OR REPLACE - drop any existing entries first (if any)
Expand Down Expand Up @@ -238,7 +239,8 @@ void SQLiteSchemaEntry::Alter(ClientContext &context, AlterInfo &info) {
AlterTable(transaction, alter.Cast<RemoveColumnInfo>());
break;
default:
throw BinderException("Unsupported ALTER TABLE type - SQLite tables only support RENAME TABLE, RENAME COLUMN, "
throw BinderException("Unsupported ALTER TABLE type - SQLite tables only "
"support RENAME TABLE, RENAME COLUMN, "
"ADD COLUMN and DROP COLUMN");
}
transaction.ClearTableEntry(info.name);
Expand Down
Loading