Fix #25018: Throw user-facing error when thread settings are NULL

Project: duckdb/duckdb | Language: C++ | Concepts: error-handling, testing-strategy | Difficulty: junior
Link: https://github.com/duckdb/duckdb/pull/25021

The problem

When users ran commands like SET threads=NULL;, DuckDB would crash with an internal error. This happened because the code tried to extract a value from a NULL setting without checking for nullness, leading to an unhandled exception and a confusing error message.

Why it's non-trivial

A naive fix might only patch the threads setting, but several other settings (e.g., external_threads, async_threads, etc.) used the same pattern and were vulnerable to the same bug. A robust solution needs to systematically check for NULL values across all relevant settings and return clear, user-facing errors instead of crashing.

The approach

The PR adds explicit NULL checks and throws meaningful exceptions for all affected settings. Here are the key code changes:

void ExternalThreadsSetting::OnSet(SettingCallbackInfo &info, Value &input) {
+	if (input.IsNull()) {
+		throw InvalidInputException("external_threads must be a positive integer");
+	}
 	auto new_external_threads = input.GetValue<uint64_t>();

What it does:
Before extracting the value, this checks if the input is NULL. If so, it throws a user-facing error instead of proceeding.

void OrderedAggregateThresholdSetting::OnSet(SettingCallbackInfo &info, Value &input) {
+	if (input.IsNull()) {
+		throw InvalidInputException("ordered_aggregate_threshold must be a positive integer");
+	}
 	const auto param = input.GetValue<uint64_t>();

What it does:
Same pattern: early NULL check for this setting, with a clear error message.

void PerfectHtThresholdSetting::OnSet(SettingCallbackInfo &info, Value &input) {
+	if (input.IsNull()) {
+		throw InvalidInputException("perfect_ht_threshold must be an integer");
+	}
 	auto bits = input.GetValue<int64_t>();

What it does:
Covers another related setting, ensuring all similar bugs are fixed.

void ThreadsSetting::SetGlobal(DatabaseInstance *db, DBConfig &config, const Value &input) {
+	if (input.IsNull()) {
+		throw InvalidInputException("threads must be a positive integer");
+	}
 	auto new_val = input.GetValue<int64_t>();

What it does:
Directly addresses the original bug for the threads setting.

void AsyncThreadsSetting::SetGlobal(DatabaseInstance *db, DBConfig &config, const Value &input) {
+	if (input.IsNull()) {
+		throw InvalidInputException("async_threads must be a positive integer");
+	}
 	auto new_val = input.GetValue<int64_t>();

What it does:
Completes the pattern for all relevant thread-related settings.

Tests were added to ensure that setting any of these to NULL triggers the correct error:

+statement error
+SET threads = NULL;
+----
+Invalid Input Error: threads must be a positive integer
+...

What it does:
Verifies that the error is now user-friendly and specific for each setting.

From the review thread

Mytherin:

Thanks but the test is in the wrong format - can you move it to the correct format

c0d3l0v3r-HeHe:

Hi @Mytherin , I've updated the tests to use the correct exact substring matching format at the bottom of the file instead of regex inside the loop, as requested. I ran the tests and all assertions pass locally. Let me know if anything else is needed!

Takeaways

• Always validate user input (including NULL) before attempting to extract or convert its value.
• Return clear, user-facing error messages instead of allowing internal exceptions or crashes to propagate.
• When fixing a bug pattern, check for and address similar issues across all related code paths, not just the one reported.