Bump clang-format and ruff versions to latest (#496)

In #98 we started using the same version of clang-format that DuckDB uses. That version is pretty ancient though. So it misses a bunch of features that we might want to use, and also has a bunch of bugs which result in sub-optimal formatting. This updates `clang-format` to the latest version, and while we're at it we also update `ruff`. To be able to reformat all existing files, this also adds a `format-all` target to our `Makefile`. The problem was called out by @dpxcc in #493, but I've been also running into this myself for a while.
duckdb · Dec 16, 2024 · eb2c6bb · eb2c6bb
1 parent 04be407
commit eb2c6bb
Show file tree

Hide file tree

Showing 16 changed files with 48 additions and 37 deletions.
diff --git a/Makefile b/Makefile
@@ -114,3 +114,7 @@ lintcheck:
 format:
 	git clang-format origin/main
 	ruff format
+
+format-all:
+	find src include -iname '*.h' -o -iname '*.cpp' -o -iname '*.c' | xargs clang-format -i
+	ruff format
diff --git a/dev_requirements.txt b/dev_requirements.txt
@@ -1,3 +1,3 @@
 -r requirements.txt
-ruff==0.6.1
-clang-format==11.0.1
+ruff==0.8.3
+clang-format==19.1.5
diff --git a/include/pgduckdb/pgduckdb_ruleutils.h b/include/pgduckdb/pgduckdb_ruleutils.h
@@ -4,6 +4,6 @@ char *pgduckdb_relation_name(Oid relid);
 char *pgduckdb_function_name(Oid function_oid);
 char *pgduckdb_get_querydef(Query *);
 char *pgduckdb_get_tabledef(Oid relation_id);
-bool  pgduckdb_is_not_default_expr(Node *node, void *context);
+bool pgduckdb_is_not_default_expr(Node *node, void *context);
 List *pgduckdb_db_and_schema(const char *postgres_schema_name, bool is_duckdb_table);
 const char *pgduckdb_db_and_schema_string(const char *postgres_schema_name, bool is_duckdb_table);
diff --git a/src/catalog/pgduckdb_table.cpp b/src/catalog/pgduckdb_table.cpp
@@ -42,7 +42,7 @@ PostgresTable::SetTableInfo(duckdb::CreateTableInfo &info, Relation rel) {
 		info.columns.AddColumn(duckdb::ColumnDefinition(col_name, duck_type));
 		/* Log column name and type */
 		pd_log(DEBUG2, "(DuckDB/SetTableInfo) Column name: %s, Type: %s --", col_name.c_str(),
-		     duck_type.ToString().c_str());
+		       duck_type.ToString().c_str());
 	}
 }
 

diff --git a/src/pg/error_data.cpp b/src/pg/error_data.cpp
@@ -5,7 +5,8 @@ extern "C" {
 }
 
 namespace pgduckdb::pg {
-const char* GetErrorDataMessage(ErrorData* error_data) {
-  return error_data->message;
+const char *
+GetErrorDataMessage(ErrorData *error_data) {
+	return error_data->message;
 }
-} // namespace pgduckdb
+} // namespace pgduckdb::pg
diff --git a/src/pg/relations.cpp b/src/pg/relations.cpp
@@ -17,7 +17,8 @@ namespace pgduckdb {
 
 #undef RelationGetDescr
 
-TupleDesc RelationGetDescr(Relation rel) {
+TupleDesc
+RelationGetDescr(Relation rel) {
 	return rel->rd_att;
 }
 
@@ -66,7 +67,8 @@ CloseRelation(Relation rel) {
 	CurrentResourceOwner = saveResourceOwner;
 }
 
-void EstimateRelSize(Relation rel, int32_t *attr_widths, BlockNumber *pages, double *tuples, double *allvisfrac) {
+void
+EstimateRelSize(Relation rel, int32_t *attr_widths, BlockNumber *pages, double *tuples, double *allvisfrac) {
 	PostgresFunctionGuard(estimate_rel_size, rel, attr_widths, pages, tuples, allvisfrac);
 }
 

diff --git a/src/pgduckdb.cpp b/src/pgduckdb.cpp
@@ -133,13 +133,15 @@ DuckdbInitGUC(void) {
 	                     "Allow DuckDB to load extensions with invalid or missing signatures",
 	                     &duckdb_allow_unsigned_extensions, PGC_SUSET);
 
-	DefineCustomVariable("duckdb.autoinstall_known_extensions",
-	                     "Whether known extensions are allowed to be automatically installed when a DuckDB query depends on them",
-	                     &duckdb_autoinstall_known_extensions, PGC_SUSET);
-
-	DefineCustomVariable("duckdb.autoload_known_extensions",
-	                     "Whether known extensions are allowed to be automatically loaded when a DuckDB query depends on them",
-	                     &duckdb_autoload_known_extensions, PGC_SUSET);
+	DefineCustomVariable(
+	    "duckdb.autoinstall_known_extensions",
+	    "Whether known extensions are allowed to be automatically installed when a DuckDB query depends on them",
+	    &duckdb_autoinstall_known_extensions, PGC_SUSET);
+
+	DefineCustomVariable(
+	    "duckdb.autoload_known_extensions",
+	    "Whether known extensions are allowed to be automatically loaded when a DuckDB query depends on them",
+	    &duckdb_autoload_known_extensions, PGC_SUSET);
 
 	DefineCustomVariable("duckdb.max_memory", "The maximum memory DuckDB can use (e.g., 1GB)", &duckdb_maximum_memory,
 	                     PGC_SUSET);

diff --git a/src/pgduckdb_background_worker.cpp b/src/pgduckdb_background_worker.cpp
@@ -115,7 +115,9 @@ force_motherduck_sync(PG_FUNCTION_ARGS) {
 		pgduckdb::SyncMotherDuckCatalogsWithPg(drop_with_cascade);
 	}
 	PG_FINALLY();
-	{ pgduckdb::doing_motherduck_sync = false; }
+	{
+		pgduckdb::doing_motherduck_sync = false;
+	}
 	PG_END_TRY();
 	SPI_finish();
 	PG_RETURN_VOID();
@@ -277,7 +279,9 @@ SPI_run_utility_command(const char *query) {
 	 */
 	BeginInternalSubTransaction(NULL);
 	PG_TRY();
-	{ ret = SPI_exec(query, 0); }
+	{
+		ret = SPI_exec(query, 0);
+	}
 	PG_CATCH();
 	{
 		MemoryContextSwitchTo(old_context);

diff --git a/src/pgduckdb_metadata_cache.cpp b/src/pgduckdb_metadata_cache.cpp
@@ -109,7 +109,7 @@ BuildDuckdbOnlyFunctions() {
 	 * each of the found functions is actually part of our extension before
 	 * caching its OID as a DuckDB-only function.
 	 */
-	const char *function_names[] = {"read_parquet", "read_csv", "iceberg_scan", "iceberg_metadata",
+	const char *function_names[] = {"read_parquet",      "read_csv",   "iceberg_scan", "iceberg_metadata",
 	                                "iceberg_snapshots", "delta_scan", "read_json"};
 
 	for (uint32_t i = 0; i < lengthof(function_names); i++) {

diff --git a/src/pgduckdb_node.cpp b/src/pgduckdb_node.cpp
@@ -115,7 +115,7 @@ ExecuteQuery(DuckdbScanState *state) {
 			pg_param = &pg_params->params[i];
 		}
 
-		if (prepared.named_param_map.count(duckdb::to_string(i + 1)) == 0){
+		if (prepared.named_param_map.count(duckdb::to_string(i + 1)) == 0) {
 			continue;
 		}
 

diff --git a/src/pgduckdb_ruleutils.cpp b/src/pgduckdb_ruleutils.cpp
@@ -381,7 +381,7 @@ pgduckdb_is_not_default_expr(Node *node, void *context) {
 		return true;
 	} else if (IsA(node, Const)) {
 		/* If location is -1, it comes from the DEFAULT clause */
-		Const *con = (Const *) node;
+		Const *con = (Const *)node;
 		if (con->location != -1) {
 			return true;
 		}

diff --git a/src/pgduckdb_table_am.cpp b/src/pgduckdb_table_am.cpp
@@ -304,8 +304,8 @@ duckdb_scan_analyze_next_block(TableScanDesc /*scan*/, BlockNumber /*blockno*/,
 #endif
 
 static bool
-duckdb_scan_analyze_next_tuple(TableScanDesc /*scan*/, TransactionId /*OldestXmin*/, double * /*liverows*/, double * /*deadrows*/,
-                               TupleTableSlot * /*slot*/) {
+duckdb_scan_analyze_next_tuple(TableScanDesc /*scan*/, TransactionId /*OldestXmin*/, double * /*liverows*/,
+                               double * /*deadrows*/, TupleTableSlot * /*slot*/) {
 	NOT_IMPLEMENTED();
 }
 

diff --git a/src/pgduckdb_types.cpp b/src/pgduckdb_types.cpp
@@ -223,7 +223,7 @@ ConvertDoubleDatum(const duckdb::Value &value) {
 
 template <class T, class OP = DecimalConversionInteger>
 void
-ConvertNumeric(const duckdb::Value& ddb_value, idx_t scale, NumericVar& result) {
+ConvertNumeric(const duckdb::Value &ddb_value, idx_t scale, NumericVar &result) {
 	result.dscale = scale;
 
 	T value = ddb_value.GetValueUnsafe<T>();

diff --git a/src/scan/heap_reader.cpp b/src/scan/heap_reader.cpp
@@ -24,7 +24,7 @@ namespace pgduckdb {
 //
 
 HeapReaderGlobalState::HeapReaderGlobalState(Relation rel)
-: m_nblocks(RelationGetNumberOfBlocks(rel)), m_last_assigned_block_number(InvalidBlockNumber) {
+    : m_nblocks(RelationGetNumberOfBlocks(rel)), m_last_assigned_block_number(InvalidBlockNumber) {
 }
 
 BlockNumber
@@ -95,7 +95,7 @@ HeapReader::ReadPageTuples(duckdb::DataChunk &output) {
 		m_read_next_page = true;
 	} else {
 		block = m_block_number;
-		if(!m_read_next_page) {
+		if (!m_read_next_page) {
 			page = BufferGetPage(m_buffer);
 		}
 	}

diff --git a/src/scan/postgres_seq_scan.cpp b/src/scan/postgres_seq_scan.cpp
@@ -82,8 +82,8 @@ duckdb::unique_ptr<duckdb::LocalTableFunctionState>
 PostgresSeqScanFunction::PostgresSeqScanInitLocal(duckdb::ExecutionContext &, duckdb::TableFunctionInitInput &,
                                                   duckdb::GlobalTableFunctionState *gstate) {
 	auto global_state = reinterpret_cast<PostgresSeqScanGlobalState *>(gstate);
-	return duckdb::make_uniq<PostgresSeqScanLocalState>(
-	    global_state->m_rel, global_state->m_heap_reader_global_state, global_state->m_global_state);
+	return duckdb::make_uniq<PostgresSeqScanLocalState>(global_state->m_rel, global_state->m_heap_reader_global_state,
+	                                                    global_state->m_global_state);
 }
 
 void

diff --git a/src/utility/copy.cpp b/src/utility/copy.cpp
@@ -118,15 +118,13 @@ CheckQueryPermissions(Query *query, const char *query_string) {
 }
 
 static char *
-CommaSeparatedQuotedList(const List *names)
-{
+CommaSeparatedQuotedList(const List *names) {
 	StringInfoData string;
-	ListCell   *l;
+	ListCell *l;
 
 	initStringInfo(&string);
 
-	foreach(l, names)
-	{
+	foreach (l, names) {
 		if (l != list_head(names))
 			appendStringInfoChar(&string, ',');
 		appendStringInfoString(&string, quote_identifier(strVal(lfirst(l))));
@@ -135,7 +133,6 @@ CommaSeparatedQuotedList(const List *names)
 	return string.data;
 }
 
-
 static void
 AppendCreateCopyOptions(StringInfo info, CopyStmt *copy_stmt) {
 	if (list_length(copy_stmt->options) == 0) {
@@ -198,7 +195,7 @@ CheckRewritten(List *rewritten) {
 		                errmsg("DO INSTEAD NOTHING rules are not supported for COPY")));
 	} else if (list_length(rewritten) > 1) {
 		/* examine queries to determine which error message to issue */
-		foreach_node (Query, q, rewritten) {
+		foreach_node(Query, q, rewritten) {
 			if (q->querySource == QSRC_QUAL_INSTEAD_RULE)
 				ereport(ERROR, (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				                errmsg("conditional DO INSTEAD rules are not supported for COPY")));
@@ -212,7 +209,8 @@ CheckRewritten(List *rewritten) {
 	}
 }
 
-bool CheckPrefix(const char* str, const char* prefix) {
+bool
+CheckPrefix(const char *str, const char *prefix) {
 	while (*prefix) {
 		if (*prefix++ != *str++) {
 			return false;
@@ -221,7 +219,7 @@ bool CheckPrefix(const char* str, const char* prefix) {
 	return true;
 }
 
-const char*
+const char *
 MakeDuckdbCopyQuery(PlannedStmt *pstmt, const char *query_string, struct QueryEnvironment *query_env) {
 	CopyStmt *copy_stmt = (CopyStmt *)pstmt->utilityStmt;
 	if (!copy_stmt->filename) {