Merge pull request #69 from Mytherin/issuefixes

Fix #44, #47, #53

.github/workflows/Windows.yml

@@ -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:

CMakeLists.txt

@@ -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}")

Makefile

@@ -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

data/sql/tpch-export.duckdb

@@ -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);

src/include/sqlite_db.hpp

@@ -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);

src/include/sqlite_scanner_extension.hpp

@@ -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" {

src/include/sqlite_stmt.hpp

@@ -13,6 +13,7 @@
 #include <cstddef>
 
 namespace duckdb {
+struct SqliteBindData;
 
 class SQLiteStatement {
 public:
@@ -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();
 };

src/include/storage/sqlite_delete.hpp

@@ -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

src/include/storage/sqlite_table_entry.hpp

@@ -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;
@@ -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

src/sqlite_db.cpp

@@ -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;
@@ -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");
@@ -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()) {

src/sqlite_extension.cpp

@@ -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() {

src/sqlite_scanner.cpp

@@ -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());
@@ -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 {
@@ -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;

src/sqlite_stmt.cpp

@@ -1,5 +1,6 @@
 #include "sqlite_stmt.hpp"
 #include "sqlite_db.hpp"
+#include "sqlite_scanner.hpp"
 
 namespace duckdb {
 
@@ -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));

src/sqlite_utils.cpp

@@ -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;
 	}

src/storage/sqlite_catalog.cpp

@@ -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;
@@ -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;
 }

src/storage/sqlite_delete.cpp

@@ -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) {
 }
 
 //===--------------------------------------------------------------------===//
@@ -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]);
@@ -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);
 }

src/storage/sqlite_schema_entry.cpp

@@ -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)
@@ -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);