Merge pull request duckdb#11546 from Tishj/client_properties_timezone

[ICU] Use the correct lookup precedence for TimeZone settings
nickgerrets · Apr 8, 2024 · b4408a8 · b4408a8
2 parents 1d85301 + b59c4f4
commit b4408a8
Show file tree

Hide file tree

Showing 14 changed files with 176 additions and 147 deletions.
diff --git a/src/include/duckdb/main/client_context.hpp b/src/include/duckdb/main/client_context.hpp
@@ -167,7 +167,7 @@ class ClientContext : public std::enable_shared_from_this<ClientContext> {
 	                                                 bool requires_valid_transaction = true);
 
 	//! Equivalent to CURRENT_SETTING(key) SQL function.
-	DUCKDB_API SettingLookupResult TryGetCurrentSetting(const std::string &key, Value &result);
+	DUCKDB_API SettingLookupResult TryGetCurrentSetting(const std::string &key, Value &result) const;
 
 	//! Returns the parser options for this client context
 	DUCKDB_API ParserOptions GetParserOptions() const;

diff --git a/src/include/duckdb/main/client_data.hpp b/src/include/duckdb/main/client_data.hpp
@@ -64,6 +64,7 @@ struct ClientData {
 
 public:
 	DUCKDB_API static ClientData &Get(ClientContext &context);
+	DUCKDB_API static const ClientData &Get(const ClientContext &context);
 };
 
 } // namespace duckdb
diff --git a/src/include/duckdb/main/config.hpp b/src/include/duckdb/main/config.hpp
@@ -60,7 +60,7 @@ typedef void (*set_global_function_t)(DatabaseInstance *db, DBConfig &config, co
 typedef void (*set_local_function_t)(ClientContext &context, const Value &parameter);
 typedef void (*reset_global_function_t)(DatabaseInstance *db, DBConfig &config);
 typedef void (*reset_local_function_t)(ClientContext &context);
-typedef Value (*get_setting_function_t)(ClientContext &context);
+typedef Value (*get_setting_function_t)(const ClientContext &context);
 
 struct ConfigurationOption {
 	const char *name;

diff --git a/src/include/duckdb/main/database.hpp b/src/include/duckdb/main/database.hpp
@@ -40,6 +40,7 @@ class DatabaseInstance : public std::enable_shared_from_this<DatabaseInstance> {
 	BufferPool &GetBufferPool() const;
 	DUCKDB_API SecretManager &GetSecretManager();
 	DUCKDB_API BufferManager &GetBufferManager();
+	DUCKDB_API const BufferManager &GetBufferManager() const;
 	DUCKDB_API DatabaseManager &GetDatabaseManager();
 	DUCKDB_API FileSystem &GetFileSystem();
 	DUCKDB_API TaskScheduler &GetScheduler();
@@ -51,11 +52,12 @@ class DatabaseInstance : public std::enable_shared_from_this<DatabaseInstance> {
 	idx_t NumberOfThreads();
 
 	DUCKDB_API static DatabaseInstance &GetDatabase(ClientContext &context);
+	DUCKDB_API static const DatabaseInstance &GetDatabase(const ClientContext &context);
 
 	DUCKDB_API const unordered_set<std::string> &LoadedExtensions();
 	DUCKDB_API bool ExtensionIsLoaded(const string &name);
 
-	DUCKDB_API SettingLookupResult TryGetCurrentSetting(const string &key, Value &result);
+	DUCKDB_API SettingLookupResult TryGetCurrentSetting(const string &key, Value &result) const;
 
 	unique_ptr<AttachedDatabase> CreateAttachedDatabase(ClientContext &context, const AttachInfo &info,
 	                                                    const string &type, AccessMode access_mode);

diff --git a/src/include/duckdb/main/settings.hpp b/src/include/duckdb/main/settings.hpp
diff --git a/src/include/duckdb/storage/buffer_manager.hpp b/src/include/duckdb/storage/buffer_manager.hpp
@@ -53,7 +53,7 @@ class BufferManager {
 	//! blocks can be evicted
 	virtual void SetLimit(idx_t limit = (idx_t)-1);
 	virtual vector<TemporaryFileInformation> GetTemporaryFiles();
-	virtual const string &GetTemporaryDirectory();
+	virtual const string &GetTemporaryDirectory() const;
 	virtual void SetTemporaryDirectory(const string &new_dir);
 	virtual bool HasTemporaryDirectory() const;
 	//! Construct a managed buffer.
@@ -66,7 +66,9 @@ class BufferManager {
 
 	// Static methods
 	DUCKDB_API static BufferManager &GetBufferManager(DatabaseInstance &db);
+	DUCKDB_API static const BufferManager &GetBufferManager(const DatabaseInstance &db);
 	DUCKDB_API static BufferManager &GetBufferManager(ClientContext &context);
+	DUCKDB_API static const BufferManager &GetBufferManager(const ClientContext &context);
 	DUCKDB_API static BufferManager &GetBufferManager(AttachedDatabase &db);
 
 	static idx_t GetAllocSize(idx_t block_size) {

diff --git a/src/include/duckdb/storage/standard_buffer_manager.hpp b/src/include/duckdb/storage/standard_buffer_manager.hpp
@@ -72,7 +72,7 @@ class StandardBufferManager : public BufferManager {
 	//! Returns a list of all temporary files
 	vector<TemporaryFileInformation> GetTemporaryFiles() final;
 
-	const string &GetTemporaryDirectory() final {
+	const string &GetTemporaryDirectory() const final {
 		return temp_directory;
 	}
 

diff --git a/src/main/client_context.cpp b/src/main/client_context.cpp
@@ -1235,7 +1235,7 @@ unique_ptr<QueryResult> ClientContext::Execute(const shared_ptr<Relation> &relat
 	return ErrorResult<MaterializedQueryResult>(ErrorData(err_str));
 }
 
-SettingLookupResult ClientContext::TryGetCurrentSetting(const std::string &key, Value &result) {
+SettingLookupResult ClientContext::TryGetCurrentSetting(const std::string &key, Value &result) const {
 	// first check the built-in settings
 	auto &db_config = DBConfig::GetConfig(*this);
 	auto option = db_config.GetOptionByName(key);
@@ -1270,17 +1270,9 @@ ParserOptions ClientContext::GetParserOptions() const {
 ClientProperties ClientContext::GetClientProperties() const {
 	string timezone = "UTC";
 	Value result;
-	// 1) Check Set Variable
-	auto &client_config = ClientConfig::GetConfig(*this);
-	auto tz_config = client_config.set_variables.find("timezone");
-	if (tz_config == client_config.set_variables.end()) {
-		// 2) Check for Default Value
-		auto default_value = db->config.extension_parameters.find("timezone");
-		if (default_value != db->config.extension_parameters.end()) {
-			timezone = default_value->second.default_value.GetValue<string>();
-		}
-	} else {
-		timezone = tz_config->second.GetValue<string>();
+
+	if (TryGetCurrentSetting("TimeZone", result)) {
+		timezone = result.ToString();
 	}
 	return {timezone, db->config.options.arrow_offset_size};
 }

diff --git a/src/main/client_data.cpp b/src/main/client_data.cpp
@@ -50,6 +50,10 @@ ClientData &ClientData::Get(ClientContext &context) {
 	return *context.client_data;
 }
 
+const ClientData &ClientData::Get(const ClientContext &context) {
+	return *context.client_data;
+}
+
 RandomEngine &RandomEngine::Get(ClientContext &context) {
 	return *ClientData::Get(context).random_engine;
 }

diff --git a/src/main/database.cpp b/src/main/database.cpp
@@ -71,6 +71,10 @@ BufferManager &BufferManager::GetBufferManager(DatabaseInstance &db) {
 	return db.GetBufferManager();
 }
 
+const BufferManager &BufferManager::GetBufferManager(const DatabaseInstance &db) {
+	return db.GetBufferManager();
+}
+
 BufferManager &BufferManager::GetBufferManager(AttachedDatabase &db) {
 	return BufferManager::GetBufferManager(db.GetDatabase());
 }
@@ -79,6 +83,10 @@ DatabaseInstance &DatabaseInstance::GetDatabase(ClientContext &context) {
 	return *context.db;
 }
 
+const DatabaseInstance &DatabaseInstance::GetDatabase(const ClientContext &context) {
+	return *context.db;
+}
+
 DatabaseManager &DatabaseInstance::GetDatabaseManager() {
 	if (!db_manager) {
 		throw InternalException("Missing DB manager");
@@ -288,6 +296,10 @@ BufferManager &DatabaseInstance::GetBufferManager() {
 	return *buffer_manager;
 }
 
+const BufferManager &DatabaseInstance::GetBufferManager() const {
+	return *buffer_manager;
+}
+
 BufferPool &DatabaseInstance::GetBufferPool() const {
 	return *config.buffer_pool;
 }
@@ -411,7 +423,7 @@ void DatabaseInstance::SetExtensionLoaded(const std::string &name) {
 	}
 }
 
-SettingLookupResult DatabaseInstance::TryGetCurrentSetting(const std::string &key, Value &result) {
+SettingLookupResult DatabaseInstance::TryGetCurrentSetting(const std::string &key, Value &result) const {
 	// check the session values
 	auto &db_config = DBConfig::GetConfig(*this);
 	const auto &global_config_map = db_config.options.set_variables;