Skip to content

Commit

Permalink
Fail compilation on warnings (#439)
Browse files Browse the repository at this point in the history 
* fail compilation on warnings (`-Werror`)
* remove unused parameters
* fix integer comparisons
  • Loading branch information
@Y--
Y-- authored Nov 25, 2024
1 parent 6e1399f commit d1c7caf
Show file tree
Hide file tree
Showing 29 changed files with 203 additions and 184 deletions.
5 changes: 2 additions & 3 deletions .github/workflows/build_and_test.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,9 +110,8 @@ jobs:

- name: Build pg_duckdb extension
id: build
run: |
pushd duckdb
make -j8 install
working-directory: duckdb
run: ERROR_ON_WARNING=1 make -j8 install

- name: Run make installcheck
id: installcheck
Expand Down
13 changes: 11 additions & 2 deletions Makefile
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,17 @@ endif
DUCKDB_LIB = libduckdb$(DLSUFFIX)
FULL_DUCKDB_LIB = third_party/duckdb/build/$(DUCKDB_BUILD_TYPE)/src/$(DUCKDB_LIB)

override PG_CPPFLAGS += -Iinclude -Ithird_party/duckdb/src/include -Ithird_party/duckdb/third_party/re2
override PG_CXXFLAGS += -std=c++17 -Wno-sign-compare -Wno-register ${DUCKDB_BUILD_CXX_FLAGS}
ERROR_ON_WARNING ?=
ifeq ($(ERROR_ON_WARNING), 1)
ERROR_ON_WARNING = -Werror
else
ERROR_ON_WARNING =
endif

COMPILER_FLAGS=-Wno-sign-compare -Wno-register -Wshadow -Wswitch -Wunused-parameter -Wunreachable-code -Wno-unknown-pragmas -Wall -Wextra ${ERROR_ON_WARNING}

override PG_CPPFLAGS += -Iinclude -isystem third_party/duckdb/src/include -isystem third_party/duckdb/third_party/re2 ${COMPILER_FLAGS}
override PG_CXXFLAGS += -std=c++17 ${DUCKDB_BUILD_CXX_FLAGS} ${COMPILER_FLAGS}

SHLIB_LINK += -Wl,-rpath,$(PG_LIB)/ -lpq -Lthird_party/duckdb/build/$(DUCKDB_BUILD_TYPE)/src -L$(PG_LIB) -lduckdb -lstdc++ -llz4

Expand Down
2 changes: 1 addition & 1 deletion include/pgduckdb/pgduckdb_ddl.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@

#include "pgduckdb/pg/declarations.hpp"

void DuckdbHandleDDL(Node *ParseTree, const char *queryString);
void DuckdbHandleDDL(Node *ParseTree);
void DuckdbTruncateTable(Oid relation_oid);
5 changes: 2 additions & 3 deletions include/pgduckdb/pgduckdb_utils.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,9 +88,8 @@ __CPPFunctionGuard__(const char *func_name, FuncArgs... args) {
Datum func_name##_cpp(PG_FUNCTION_ARGS); \
Datum func_name(PG_FUNCTION_ARGS) { \
return InvokeCPPFunc(func_name##_cpp, fcinfo); \
} \
Datum func_name##_cpp(PG_FUNCTION_ARGS)

} \
Datum func_name##_cpp(PG_FUNCTION_ARGS __attribute__((unused)))

duckdb::unique_ptr<duckdb::QueryResult> DuckDBQueryOrThrow(duckdb::ClientContext &context, const std::string &query);

Expand Down
2 changes: 1 addition & 1 deletion include/pgduckdb/scan/postgres_scan.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ class PostgresScanLocalState {
}
}

int m_output_vector_size;
uint32_t m_output_vector_size;
bool m_exhausted_scan;
std::vector<Datum, DuckDBMallocator<Datum>> values;
std::vector<bool, DuckDBMallocator<bool>> nulls;
Expand Down
4 changes: 2 additions & 2 deletions include/pgduckdb/types/decimal.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ struct DecimalConversionInteger {

template <class T>
static T
Finalize(const NumericVar &numeric, T result) {
Finalize(const NumericVar &, T result) {
return result;
}
};
Expand Down Expand Up @@ -349,7 +349,7 @@ struct DecimalConversionHugeint {
}

static hugeint_t
Finalize(const NumericVar &numeric, hugeint_t result) {
Finalize(const NumericVar &, hugeint_t result) {
return result;
}
};
Expand Down
2 changes: 1 addition & 1 deletion include/pgduckdb/utility/allocator.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ struct DuckDBMallocator {
}

void
deallocate(T *p, std::size_t n) noexcept {
deallocate(T *p, std::size_t) noexcept {
duckdb_free(p);
}
};
Expand Down
53 changes: 24 additions & 29 deletions src/catalog/pgduckdb_catalog.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,24 +9,21 @@

namespace pgduckdb {

PostgresCatalog::PostgresCatalog(duckdb::AttachedDatabase &db, const duckdb::string &connection_string,
duckdb::AccessMode access_mode)
: Catalog(db), path(connection_string), access_mode(access_mode) {
PostgresCatalog::PostgresCatalog(duckdb::AttachedDatabase &_db, const duckdb::string &connection_string,
duckdb::AccessMode _access_mode)
: Catalog(_db), path(connection_string), access_mode(_access_mode) {
}

duckdb::unique_ptr<duckdb::Catalog>
PostgresCatalog::Attach(duckdb::StorageExtensionInfo *storage_info_p, duckdb::ClientContext &context,
duckdb::AttachedDatabase &db, const duckdb::string &name, duckdb::AttachInfo &info,
duckdb::AccessMode access_mode) {
auto connection_string = info.path;
return duckdb::make_uniq<PostgresCatalog>(db, connection_string, access_mode);
PostgresCatalog::Attach(duckdb::StorageExtensionInfo *, duckdb::ClientContext &, duckdb::AttachedDatabase &db,
const duckdb::string &, duckdb::AttachInfo &info, duckdb::AccessMode access_mode) {
return duckdb::make_uniq<PostgresCatalog>(db, info.path, access_mode);
}

// ------------------ Catalog API ---------------------

void
PostgresCatalog::Initialize(bool load_builtin) {
return;
PostgresCatalog::Initialize(bool /*load_builtin*/) {
}

duckdb::string
Expand All @@ -35,58 +32,56 @@ PostgresCatalog::GetCatalogType() {
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresCatalog::CreateSchema(duckdb::CatalogTransaction transaction, duckdb::CreateSchemaInfo &info) {
PostgresCatalog::CreateSchema(duckdb::CatalogTransaction, duckdb::CreateSchemaInfo &) {
throw duckdb::NotImplementedException("CreateSchema not supported yet");
}

duckdb::optional_ptr<duckdb::SchemaCatalogEntry>
PostgresCatalog::GetSchema(duckdb::CatalogTransaction transaction, const duckdb::string &schema_name,
duckdb::OnEntryNotFound if_not_found, duckdb::QueryErrorContext error_context) {
auto &pg_transaction = transaction.transaction->Cast<PostgresTransaction>();
PostgresCatalog::GetSchema(duckdb::CatalogTransaction catalog_transaction, const duckdb::string &schema_name,
duckdb::OnEntryNotFound, duckdb::QueryErrorContext) {
auto &pg_transaction = catalog_transaction.transaction->Cast<PostgresTransaction>();
auto res = pg_transaction.GetCatalogEntry(duckdb::CatalogType::SCHEMA_ENTRY, schema_name, "");
D_ASSERT(res);
D_ASSERT(res->type == duckdb::CatalogType::SCHEMA_ENTRY);
return (duckdb::SchemaCatalogEntry *)res.get();
}

void
PostgresCatalog::ScanSchemas(duckdb::ClientContext &context,
std::function<void(duckdb::SchemaCatalogEntry &)> callback) {
return;
PostgresCatalog::ScanSchemas(duckdb::ClientContext &, std::function<void(duckdb::SchemaCatalogEntry &)>) {
}

duckdb::unique_ptr<duckdb::PhysicalOperator>
PostgresCatalog::PlanCreateTableAs(duckdb::ClientContext &context, duckdb::LogicalCreateTable &op,
duckdb::unique_ptr<duckdb::PhysicalOperator> plan) {
PostgresCatalog::PlanCreateTableAs(duckdb::ClientContext &, duckdb::LogicalCreateTable &,
duckdb::unique_ptr<duckdb::PhysicalOperator>) {
throw duckdb::NotImplementedException("PlanCreateTableAs not supported yet");
}

duckdb::unique_ptr<duckdb::PhysicalOperator>
PostgresCatalog::PlanInsert(duckdb::ClientContext &context, duckdb::LogicalInsert &op,
duckdb::unique_ptr<duckdb::PhysicalOperator> plan) {
PostgresCatalog::PlanInsert(duckdb::ClientContext &, duckdb::LogicalInsert &,
duckdb::unique_ptr<duckdb::PhysicalOperator>) {
throw duckdb::NotImplementedException("PlanInsert not supported yet");
}

duckdb::unique_ptr<duckdb::PhysicalOperator>
PostgresCatalog::PlanDelete(duckdb::ClientContext &context, duckdb::LogicalDelete &op,
duckdb::unique_ptr<duckdb::PhysicalOperator> plan) {
PostgresCatalog::PlanDelete(duckdb::ClientContext &, duckdb::LogicalDelete &,
duckdb::unique_ptr<duckdb::PhysicalOperator>) {
throw duckdb::NotImplementedException("PlanDelete not supported yet");
}

duckdb::unique_ptr<duckdb::PhysicalOperator>
PostgresCatalog::PlanUpdate(duckdb::ClientContext &context, duckdb::LogicalUpdate &op,
duckdb::unique_ptr<duckdb::PhysicalOperator> plan) {
PostgresCatalog::PlanUpdate(duckdb::ClientContext &, duckdb::LogicalUpdate &,
duckdb::unique_ptr<duckdb::PhysicalOperator>) {
throw duckdb::NotImplementedException("PlanUpdate not supported yet");
}

duckdb::unique_ptr<duckdb::LogicalOperator>
PostgresCatalog::BindCreateIndex(duckdb::Binder &binder, duckdb::CreateStatement &stmt,
duckdb::TableCatalogEntry &table, duckdb::unique_ptr<duckdb::LogicalOperator> plan) {
PostgresCatalog::BindCreateIndex(duckdb::Binder &, duckdb::CreateStatement &, duckdb::TableCatalogEntry &,
duckdb::unique_ptr<duckdb::LogicalOperator>) {
throw duckdb::NotImplementedException("BindCreateIndex not supported yet");
}

duckdb::DatabaseSize
PostgresCatalog::GetDatabaseSize(duckdb::ClientContext &context) {
PostgresCatalog::GetDatabaseSize(duckdb::ClientContext &) {
throw duckdb::NotImplementedException("GetDatabaseSize not supported yet");
}

Expand All @@ -101,7 +96,7 @@ PostgresCatalog::GetDBPath() {
}

void
PostgresCatalog::DropSchema(duckdb::ClientContext &context, duckdb::DropInfo &info) {
PostgresCatalog::DropSchema(duckdb::ClientContext &, duckdb::DropInfo &) {
throw duckdb::NotImplementedException("DropSchema not supported yet");
}

Expand Down
43 changes: 20 additions & 23 deletions src/catalog/pgduckdb_schema.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,86 +7,83 @@

namespace pgduckdb {

PostgresSchema::PostgresSchema(duckdb::Catalog &catalog, duckdb::CreateSchemaInfo &info, Snapshot snapshot)
: SchemaCatalogEntry(catalog, info), snapshot(snapshot), catalog(catalog) {
PostgresSchema::PostgresSchema(duckdb::Catalog &_catalog, duckdb::CreateSchemaInfo &_info, Snapshot _snapshot)
: SchemaCatalogEntry(_catalog, _info), snapshot(_snapshot), catalog(_catalog) {
}

void
PostgresSchema::Scan(duckdb::ClientContext &context, duckdb::CatalogType type,
const std::function<void(CatalogEntry &)> &callback) {
return;
PostgresSchema::Scan(duckdb::ClientContext &, duckdb::CatalogType, const std::function<void(CatalogEntry &)> &) {
}

void
PostgresSchema::Scan(duckdb::CatalogType type, const std::function<void(duckdb::CatalogEntry &)> &callback) {
PostgresSchema::Scan(duckdb::CatalogType, const std::function<void(duckdb::CatalogEntry &)> &) {
throw duckdb::NotImplementedException("Scan(no context) not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateIndex(duckdb::CatalogTransaction transaction, duckdb::CreateIndexInfo &info,
duckdb::TableCatalogEntry &table) {
PostgresSchema::CreateIndex(duckdb::CatalogTransaction, duckdb::CreateIndexInfo &, duckdb::TableCatalogEntry &) {
throw duckdb::NotImplementedException("CreateIndex not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateFunction(duckdb::CatalogTransaction transaction, duckdb::CreateFunctionInfo &info) {
PostgresSchema::CreateFunction(duckdb::CatalogTransaction, duckdb::CreateFunctionInfo &) {
throw duckdb::NotImplementedException("CreateFunction not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateTable(duckdb::CatalogTransaction transaction, duckdb::BoundCreateTableInfo &info) {
PostgresSchema::CreateTable(duckdb::CatalogTransaction, duckdb::BoundCreateTableInfo &) {
throw duckdb::NotImplementedException("CreateTable not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateView(duckdb::CatalogTransaction transaction, duckdb::CreateViewInfo &info) {
PostgresSchema::CreateView(duckdb::CatalogTransaction, duckdb::CreateViewInfo &) {
throw duckdb::NotImplementedException("CreateView not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateSequence(duckdb::CatalogTransaction transaction, duckdb::CreateSequenceInfo &info) {
PostgresSchema::CreateSequence(duckdb::CatalogTransaction, duckdb::CreateSequenceInfo &) {
throw duckdb::NotImplementedException("CreateSequence not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateTableFunction(duckdb::CatalogTransaction transaction, duckdb::CreateTableFunctionInfo &info) {
PostgresSchema::CreateTableFunction(duckdb::CatalogTransaction, duckdb::CreateTableFunctionInfo &) {
throw duckdb::NotImplementedException("CreateTableFunction not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateCopyFunction(duckdb::CatalogTransaction transaction, duckdb::CreateCopyFunctionInfo &info) {
PostgresSchema::CreateCopyFunction(duckdb::CatalogTransaction, duckdb::CreateCopyFunctionInfo &) {
throw duckdb::NotImplementedException("CreateCopyFunction not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreatePragmaFunction(duckdb::CatalogTransaction transaction, duckdb::CreatePragmaFunctionInfo &info) {
PostgresSchema::CreatePragmaFunction(duckdb::CatalogTransaction, duckdb::CreatePragmaFunctionInfo &) {
throw duckdb::NotImplementedException("CreatePragmaFunction not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateCollation(duckdb::CatalogTransaction transaction, duckdb::CreateCollationInfo &info) {
PostgresSchema::CreateCollation(duckdb::CatalogTransaction, duckdb::CreateCollationInfo &) {
throw duckdb::NotImplementedException("CreateCollation not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::CreateType(duckdb::CatalogTransaction transaction, duckdb::CreateTypeInfo &info) {
PostgresSchema::CreateType(duckdb::CatalogTransaction, duckdb::CreateTypeInfo &) {
throw duckdb::NotImplementedException("CreateType not supported yet");
}

duckdb::optional_ptr<duckdb::CatalogEntry>
PostgresSchema::GetEntry(duckdb::CatalogTransaction transaction, duckdb::CatalogType type,
const duckdb::string &entry_name) {
auto &pg_transaction = transaction.transaction->Cast<PostgresTransaction>();
return pg_transaction.GetCatalogEntry(type, name, entry_name);
PostgresSchema::GetEntry(duckdb::CatalogTransaction _catalog_transaction, duckdb::CatalogType _type,
const duckdb::string &_entry_name) {
auto &pg_transaction = _catalog_transaction.transaction->Cast<PostgresTransaction>();
return pg_transaction.GetCatalogEntry(_type, name, _entry_name);
}

void
PostgresSchema::DropEntry(duckdb::ClientContext &context, duckdb::DropInfo &info) {
PostgresSchema::DropEntry(duckdb::ClientContext &, duckdb::DropInfo &) {
throw duckdb::NotImplementedException("DropEntry not supported yet");
}

void
PostgresSchema::Alter(duckdb::CatalogTransaction transaction, duckdb::AlterInfo &info) {
PostgresSchema::Alter(duckdb::CatalogTransaction, duckdb::AlterInfo &) {
throw duckdb::NotImplementedException("Alter not supported yet");
}

Expand Down
3 changes: 1 addition & 2 deletions src/catalog/pgduckdb_storage.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,7 @@
namespace pgduckdb {

static duckdb::unique_ptr<duckdb::TransactionManager>
CreateTransactionManager(duckdb::StorageExtensionInfo *storage_info, duckdb::AttachedDatabase &db,
duckdb::Catalog &catalog) {
CreateTransactionManager(duckdb::StorageExtensionInfo *, duckdb::AttachedDatabase &db, duckdb::Catalog &catalog) {
return duckdb::make_uniq<PostgresTransactionManager>(db, catalog.Cast<PostgresCatalog>());
}

Expand Down
22 changes: 11 additions & 11 deletions src/catalog/pgduckdb_table.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,10 @@

namespace pgduckdb {

PostgresTable::PostgresTable(duckdb::Catalog &catalog, duckdb::SchemaCatalogEntry &schema,
duckdb::CreateTableInfo &info, Relation rel, Cardinality cardinality, Snapshot snapshot)
: duckdb::TableCatalogEntry(catalog, schema, info), rel(rel), cardinality(cardinality), snapshot(snapshot) {
PostgresTable::PostgresTable(duckdb::Catalog &_catalog, duckdb::SchemaCatalogEntry &_schema,
duckdb::CreateTableInfo &_info, Relation _rel, Cardinality _cardinality,
Snapshot _snapshot)
: duckdb::TableCatalogEntry(_catalog, _schema, _info), rel(_rel), cardinality(_cardinality), snapshot(_snapshot) {
}

PostgresTable::~PostgresTable() {
Expand Down Expand Up @@ -58,26 +59,25 @@ PostgresTable::GetTableCardinality(Relation rel) {
// PostgresHeapTable
//===--------------------------------------------------------------------===//

PostgresHeapTable::PostgresHeapTable(duckdb::Catalog &catalog, duckdb::SchemaCatalogEntry &schema,
duckdb::CreateTableInfo &info, Relation rel, Cardinality cardinality,
Snapshot snapshot)
: PostgresTable(catalog, schema, info, rel, cardinality, snapshot) {
PostgresHeapTable::PostgresHeapTable(duckdb::Catalog &_catalog, duckdb::SchemaCatalogEntry &_schema,
duckdb::CreateTableInfo &_info, Relation _rel, Cardinality _cardinality,
Snapshot _snapshot)
: PostgresTable(_catalog, _schema, _info, _rel, _cardinality, _snapshot) {
}

duckdb::unique_ptr<duckdb::BaseStatistics>
PostgresHeapTable::GetStatistics(duckdb::ClientContext &context, duckdb::column_t column_id) {
PostgresHeapTable::GetStatistics(duckdb::ClientContext &, duckdb::column_t) {
throw duckdb::NotImplementedException("GetStatistics not supported yet");
}

duckdb::TableFunction
PostgresHeapTable::GetScanFunction(duckdb::ClientContext &context,
duckdb::unique_ptr<duckdb::FunctionData> &bind_data) {
PostgresHeapTable::GetScanFunction(duckdb::ClientContext &, duckdb::unique_ptr<duckdb::FunctionData> &bind_data) {
bind_data = duckdb::make_uniq<PostgresSeqScanFunctionData>(rel, cardinality, snapshot);
return PostgresSeqScanFunction();
}

duckdb::TableStorageInfo
PostgresHeapTable::GetStorageInfo(duckdb::ClientContext &context) {
PostgresHeapTable::GetStorageInfo(duckdb::ClientContext &) {
throw duckdb::NotImplementedException("GetStorageInfo not supported yet");
}

Expand Down
10 changes: 5 additions & 5 deletions src/catalog/pgduckdb_transaction.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@ ClosePostgresRelations(duckdb::ClientContext &context) {
context_state->QueryEnd();
}

PostgresTransaction::PostgresTransaction(duckdb::TransactionManager &manager, duckdb::ClientContext &context,
PostgresCatalog &catalog, Snapshot snapshot)
: duckdb::Transaction(manager, context), catalog(catalog), snapshot(snapshot) {
PostgresTransaction::PostgresTransaction(duckdb::TransactionManager &_manager, duckdb::ClientContext &_context,
PostgresCatalog &_catalog, Snapshot _snapshot)
: duckdb::Transaction(_manager, _context), catalog(_catalog), snapshot(_snapshot) {
}

PostgresTransaction::~PostgresTransaction() {
}

SchemaItems::SchemaItems(duckdb::unique_ptr<PostgresSchema> &&schema, const duckdb::string &name)
: name(name), schema(std::move(schema)) {
SchemaItems::SchemaItems(duckdb::unique_ptr<PostgresSchema> &&_schema, const duckdb::string &_name)
: name(_name), schema(std::move(_schema)) {
}

duckdb::optional_ptr<duckdb::CatalogEntry>
Expand Down
Loading

0 comments on commit d1c7caf

Please sign in to comment.