Remove logging of query strings in WAL for ALTER TABLE (the only stat…

…ement type that still logged query strings)

src/catalog/catalog_set.cpp

@@ -5,6 +5,8 @@
 #include "duckdb/transaction/transaction_manager.hpp"
 #include "duckdb/transaction/transaction.hpp"
 #include "duckdb/main/client_context.hpp"
+#include "duckdb/common/serializer/buffered_serializer.hpp"
+#include "duckdb/parser/parsed_data/alter_table_info.hpp"
 
 using namespace duckdb;
 using namespace std;
@@ -98,8 +100,13 @@ bool CatalogSet::AlterEntry(ClientContext &context, const string &name, AlterInf
 	value->child->parent = value.get();
 	value->set = this;
 
+	// serialize the AlterInfo into a temporary buffer
+	BufferedSerializer serializer;
+	alter_info->Serialize(serializer);
+	BinaryData serialized_alter = serializer.GetData();
+
 	// push the old entry in the undo buffer for this transaction
-	transaction.PushCatalogEntry(value->child.get());
+	transaction.PushCatalogEntry(value->child.get(), serialized_alter.data.get(), serialized_alter.size);
 	data[name] = move(value);
 
 	return true;

src/include/duckdb/common/enums/wal_type.hpp

@@ -29,6 +29,8 @@ enum class WALType : uint8_t {
 	CREATE_SEQUENCE = 8,
 	DROP_SEQUENCE = 9,
 	SEQUENCE_VALUE = 10,
+
+	ALTER_INFO = 20,
 	// -----------------------------
 	// Data
 	// -----------------------------
@@ -37,10 +39,6 @@ enum class WALType : uint8_t {
 	DELETE_TUPLE = 27,
 	UPDATE_TUPLE = 28,
 	// -----------------------------
-	// Query
-	// -----------------------------
-	QUERY = 50,
-	// -----------------------------
 	// Flush
 	// -----------------------------
 	WAL_FLUSH = 100

src/include/duckdb/parser/parsed_data/alter_table_info.hpp

@@ -15,35 +15,47 @@ namespace duckdb {
 enum class AlterType : uint8_t { INVALID = 0, ALTER_TABLE = 1 };
 
 struct AlterInfo {
-	AlterType type;
-
 	AlterInfo(AlterType type) : type(type) {
 	}
+	virtual ~AlterInfo(){}
+
+	AlterType type;
+
+	virtual void Serialize(Serializer &serializer);
+	static unique_ptr<AlterInfo> Deserialize(Deserializer &source);
 };
 
 enum class AlterTableType : uint8_t { INVALID = 0, RENAME_COLUMN = 1 };
 
 struct AlterTableInfo : public AlterInfo {
+	AlterTableInfo(AlterTableType type, string schema, string table)
+	    : AlterInfo(AlterType::ALTER_TABLE), alter_table_type(type), schema(schema), table(table) {
+	}
+	virtual ~AlterTableInfo() override {}
+
 	AlterTableType alter_table_type;
 	//! Schema name to alter to
 	string schema;
 	//! Table name to alter to
 	string table;
 
-	AlterTableInfo(AlterTableType type, string schema, string table)
-	    : AlterInfo(AlterType::ALTER_TABLE), alter_table_type(type), schema(schema), table(table) {
-	}
+	virtual void Serialize(Serializer &serializer) override;
+	static unique_ptr<AlterInfo> Deserialize(Deserializer &source);
 };
 
 struct RenameColumnInfo : public AlterTableInfo {
+	RenameColumnInfo(string schema, string table, string name, string new_name)
+	    : AlterTableInfo(AlterTableType::RENAME_COLUMN, schema, table), name(name), new_name(new_name) {
+	}
+	~RenameColumnInfo() override {}
+
 	//! Column old name
 	string name;
 	//! Column new name
 	string new_name;
 
-	RenameColumnInfo(string schema, string table, string name, string new_name)
-	    : AlterTableInfo(AlterTableType::RENAME_COLUMN, schema, table), name(name), new_name(new_name) {
-	}
+	void Serialize(Serializer &serializer) override;
+	static unique_ptr<AlterInfo> Deserialize(Deserializer &source, string schema, string table);
 };
 
 } // namespace duckdb

src/include/duckdb/storage/write_ahead_log.hpp

@@ -16,6 +16,8 @@
 
 namespace duckdb {
 
+struct AlterInfo;
+
 class BufferedSerializer;
 class Catalog;
 class DuckDB;
@@ -62,12 +64,12 @@ class WriteAheadLog {
 	//! Sets the table used for subsequent insert/delete/update commands
 	void WriteSetTable(string &schema, string &table);
 
+	void WriteAlter(AlterInfo &info);
+
 	void WriteInsert(DataChunk &chunk);
 	void WriteDelete(DataChunk &chunk);
 	void WriteUpdate(DataChunk &chunk, column_t col_idx);
 
-	void WriteQuery(string &query);
-
 	//! Truncate the WAL to a previous size, and clear anything currently set in the writer
 	void Truncate(int64_t size);
 	void Flush();

src/include/duckdb/transaction/commit_state.hpp

@@ -40,7 +40,7 @@ class CommitState {
 private:
 	void SwitchTable(DataTable *table, UndoFlags new_op);
 
-	void WriteCatalogEntry(CatalogEntry *entry);
+	void WriteCatalogEntry(CatalogEntry *entry, data_ptr_t extra_data);
 	void WriteDelete(DeleteInfo *info);
 	void WriteUpdate(UpdateInfo *info);
 

src/include/duckdb/transaction/transaction.hpp

@@ -57,9 +57,7 @@ class Transaction {
 	bool is_invalidated;
 
 public:
-	void PushCatalogEntry(CatalogEntry *entry);
-	//! Push a query into the undo buffer
-	void PushQuery(string query);
+	void PushCatalogEntry(CatalogEntry *entry, data_ptr_t extra_data = nullptr, index_t extra_data_size = 0);
 
 	//! Commit the current transaction with the given commit identifier. Returns true if the transaction commit was successful, or false if it was aborted.
 	bool Commit(WriteAheadLog *log, transaction_t commit_id) noexcept;

src/main/client_context.cpp

@@ -158,24 +158,15 @@ unique_ptr<QueryResult> ClientContext::ExecuteStatementInternal(string query, un
 	}
 	StatementType statement_type = statement->type;
 	bool create_stream_result = statement_type == StatementType::SELECT && allow_stream_result;
-	// for some statements, we log the literal query string in the WAL
-	bool log_query_string = statement_type == StatementType::ALTER;
 
 	profiler.StartPhase("planner");
 	Planner planner(*this);
 	planner.CreatePlan(move(statement));
 	if (!planner.plan) {
-		// we have to log here because some queries are executed in the planner
-		// return an empty result
-		if (log_query_string) {
-			ActiveTransaction().PushQuery(query);
-		}
 		return make_unique<MaterializedQueryResult>(statement_type);
 	}
 	profiler.EndPhase();
 
-	assert(!log_query_string);
-
 	auto plan = move(planner.plan);
 	// extract the result column names from the plan
 	auto names = planner.names;

src/parser/CMakeLists.txt

@@ -3,6 +3,7 @@ include_directories(../../third_party/libpg_query)
 
 add_subdirectory(constraints)
 add_subdirectory(expression)
+add_subdirectory(parsed_data)
 add_subdirectory(query_node)
 add_subdirectory(statement)
 add_subdirectory(tableref)

src/storage/wal_replay.cpp

@@ -6,6 +6,7 @@
 #include "duckdb/catalog/catalog_entry/view_catalog_entry.hpp"
 #include "duckdb/main/client_context.hpp"
 #include "duckdb/main/database.hpp"
+#include "duckdb/parser/parsed_data/alter_table_info.hpp"
 #include "duckdb/parser/parsed_data/drop_info.hpp"
 #include "duckdb/parser/parsed_data/create_schema_info.hpp"
 #include "duckdb/parser/parsed_data/create_table_info.hpp"
@@ -33,6 +34,7 @@ class ReplayState {
 private:
 	void ReplayCreateTable();
 	void ReplayDropTable();
+	void ReplayAlter();
 
 	void ReplayCreateView();
 	void ReplayDropView();
@@ -48,8 +50,6 @@ class ReplayState {
 	void ReplayInsert();
 	void ReplayDelete();
 	void ReplayUpdate();
-
-	void ReplayQuery();
 };
 
 void WriteAheadLog::Replay(DuckDB &database, string &path) {
@@ -109,6 +109,9 @@ void ReplayState::ReplayEntry(WALType entry_type) {
 	case WALType::DROP_TABLE:
 		ReplayDropTable();
 		break;
+	case WALType::ALTER_INFO:
+		ReplayAlter();
+		break;
 	case WALType::CREATE_VIEW:
 		ReplayCreateView();
 		break;
@@ -142,9 +145,6 @@ void ReplayState::ReplayEntry(WALType entry_type) {
 	case WALType::UPDATE_TUPLE:
 		ReplayUpdate();
 		break;
-	case WALType::QUERY:
-		ReplayQuery();
-		break;
 	default:
 		throw Exception("Invalid WAL entry type!");
 	}
@@ -173,6 +173,15 @@ void ReplayState::ReplayDropTable() {
 	db.catalog->DropTable(context.ActiveTransaction(), &info);
 }
 
+void ReplayState::ReplayAlter() {
+	auto info = AlterInfo::Deserialize(source);
+	if (info->type != AlterType::ALTER_TABLE) {
+		throw Exception("Expected ALTER TABLE!");
+	}
+
+	db.catalog->AlterTable(context, (AlterTableInfo*) info.get());
+}
+
 //===--------------------------------------------------------------------===//
 // Replay View
 //===--------------------------------------------------------------------===//
@@ -303,13 +312,3 @@ void ReplayState::ReplayUpdate() {
 	// now perform the update
 	current_table->storage->Update(*current_table, context, row_ids, column_ids, chunk);
 }
-
-//===--------------------------------------------------------------------===//
-// Query
-//===--------------------------------------------------------------------===//
-void ReplayState::ReplayQuery() {
-	// read the query
-	auto query = source.Read<string>();
-
-	context.Query(query, false);
-}

src/storage/write_ahead_log.cpp

@@ -3,6 +3,7 @@
 #include "duckdb/catalog/catalog_entry/schema_catalog_entry.hpp"
 #include "duckdb/catalog/catalog_entry/table_catalog_entry.hpp"
 #include "duckdb/catalog/catalog_entry/view_catalog_entry.hpp"
+#include "duckdb/parser/parsed_data/alter_table_info.hpp"
 #include <cstring>
 
 using namespace duckdb;
@@ -131,11 +132,11 @@ void WriteAheadLog::WriteUpdate(DataChunk &chunk, column_t col_idx) {
 }
 
 //===--------------------------------------------------------------------===//
-// QUERY
+// Write ALTER Statement
 //===--------------------------------------------------------------------===//
-void WriteAheadLog::WriteQuery(string &query) {
-	writer->Write<WALType>(WALType::QUERY);
-	writer->WriteString(query);
+void WriteAheadLog::WriteAlter(AlterInfo &info) {
+	writer->Write<WALType>(WALType::ALTER_INFO);
+	info.Serialize(*writer);
 }
 
 //===--------------------------------------------------------------------===//

src/transaction/commit_state.cpp

@@ -5,6 +5,8 @@
 #include "duckdb/storage/data_table.hpp"
 #include "duckdb/storage/write_ahead_log.hpp"
 #include "duckdb/storage/uncompressed_segment.hpp"
+#include "duckdb/common/serializer/buffered_deserializer.hpp"
+#include "duckdb/parser/parsed_data/alter_table_info.hpp"
 
 using namespace duckdb;
 using namespace std;
@@ -21,20 +23,28 @@ void CommitState::SwitchTable(DataTable *table, UndoFlags new_op) {
 	}
 }
 
-void CommitState::WriteCatalogEntry(CatalogEntry *entry) {
+void CommitState::WriteCatalogEntry(CatalogEntry *entry, data_ptr_t dataptr) {
 	assert(log);
 	// look at the type of the parent entry
 	auto parent = entry->parent;
 	switch (parent->type) {
 	case CatalogType::TABLE:
-		if (entry->type == CatalogType::TABLE) {
-			// ALTER TABLE statement, skip it
-			return;
-		}
 		if (parent->temporary) {
 			return;
 		}
-		log->WriteCreateTable((TableCatalogEntry *)parent);
+		if (entry->type == CatalogType::TABLE) {
+			// ALTER TABLE statement, read the extra data after the entry
+			auto extra_data_size = *((index_t*) dataptr);
+			auto extra_data = (data_ptr_t) (dataptr + sizeof(index_t));
+			// deserialize it
+			BufferedDeserializer source(extra_data, extra_data_size);
+			auto info = AlterInfo::Deserialize(source);
+			// write the alter table in the log
+			log->WriteAlter(*info);
+		} else {
+			// CREATE TABLE statement
+			log->WriteCreateTable((TableCatalogEntry *)parent);
+		}
 		break;
 	case CatalogType::SCHEMA:
 		if (entry->type == CatalogType::SCHEMA) {
@@ -134,7 +144,7 @@ template <bool HAS_LOG> void CommitState::CommitEntry(UndoFlags type, data_ptr_t
 
 		if (HAS_LOG) {
 			// push the catalog update to the WAL
-			WriteCatalogEntry(catalog_entry);
+			WriteCatalogEntry(catalog_entry, data + sizeof(CatalogEntry*));
 		}
 		break;
 	}
@@ -158,13 +168,6 @@ template <bool HAS_LOG> void CommitState::CommitEntry(UndoFlags type, data_ptr_t
 		info->version_number = commit_id;
 		break;
 	}
-	case UndoFlags::QUERY: {
-		if (HAS_LOG) {
-			string query = string((char *)data);
-			log->WriteQuery(query);
-		}
-		break;
-	}
 	case UndoFlags::DATA:
 		break;
 	default:

src/transaction/transaction.cpp

@@ -15,10 +15,23 @@
 using namespace duckdb;
 using namespace std;
 
-void Transaction::PushCatalogEntry(CatalogEntry *entry) {
-	// store only the pointer to the catalog entry
-	CatalogEntry **blob = (CatalogEntry **)undo_buffer.CreateEntry(UndoFlags::CATALOG_ENTRY, sizeof(CatalogEntry *));
-	*blob = entry;
+void Transaction::PushCatalogEntry(CatalogEntry *entry, data_ptr_t extra_data, index_t extra_data_size) {
+	index_t alloc_size = sizeof(CatalogEntry*);
+	if (extra_data_size > 0) {
+		alloc_size += extra_data_size + sizeof(index_t);
+	}
+	auto baseptr = undo_buffer.CreateEntry(UndoFlags::CATALOG_ENTRY, alloc_size);
+	// store the pointer to the catalog entry
+	*((CatalogEntry**) baseptr) = entry;
+	if (extra_data_size > 0) {
+		// copy the extra data behind the catalog entry pointer (if any)
+		baseptr += sizeof(CatalogEntry*);
+		// first store the extra data size
+		*((index_t*) baseptr) = extra_data_size;
+		baseptr += sizeof(index_t);
+		// then copy over the actual data
+		memcpy(baseptr, extra_data, extra_data_size);
+	}
 }
 
 void Transaction::PushDelete(ChunkInfo *vinfo, row_t rows[], index_t count, index_t base_row) {
@@ -51,11 +64,6 @@ UpdateInfo *Transaction::CreateUpdateInfo(index_t type_size, index_t entries) {
 	return update_info;
 }
 
-void Transaction::PushQuery(string query) {
-	char *blob = (char *)undo_buffer.CreateEntry(UndoFlags::QUERY, query.size() + 1);
-	strcpy(blob, query.c_str());
-}
-
 bool Transaction::Commit(WriteAheadLog *log, transaction_t commit_id) noexcept {
 	this->commit_id = commit_id;