From 5fdf856ee6f181da0edb9382a19c449f39961852 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Thu, 27 Jun 2024 11:33:42 +0200 Subject: [PATCH 1/6] Maintain prepared statement types by adding an explicit cast --- .../expression/bind_parameter_expression.cpp | 8 ++++++-- test/sql/prepared/prepare_maintain_types.test | 19 +++++++++++++++++++ 2 files changed, 25 insertions(+), 2 deletions(-) create mode 100644 test/sql/prepared/prepare_maintain_types.test diff --git a/src/planner/binder/expression/bind_parameter_expression.cpp b/src/planner/binder/expression/bind_parameter_expression.cpp index d46cf0235f3a..04e24211d452 100644 --- a/src/planner/binder/expression/bind_parameter_expression.cpp +++ b/src/planner/binder/expression/bind_parameter_expression.cpp @@ -1,5 +1,6 @@ #include "duckdb/parser/expression/parameter_expression.hpp" #include "duckdb/planner/binder.hpp" +#include "duckdb/planner/expression/bound_cast_expression.hpp" #include "duckdb/planner/expression/bound_constant_expression.hpp" #include "duckdb/planner/expression/bound_parameter_expression.hpp" #include "duckdb/planner/expression_binder.hpp" @@ -19,10 +20,13 @@ BindResult ExpressionBinder::BindExpression(ParameterExpression &expr, idx_t dep if (param_data_it != parameter_data.end()) { // it has! emit a constant directly auto &data = param_data_it->second; + auto return_type = binder.parameters->GetReturnType(parameter_id); auto constant = make_uniq(data.GetValue()); constant->alias = expr.alias; - constant->return_type = binder.parameters->GetReturnType(parameter_id); - return BindResult(std::move(constant)); + constant->return_type = return_type; + + auto cast = BoundCastExpression::AddCastToType(context, std::move(constant), return_type); + return BindResult(std::move(cast)); } auto bound_parameter = binder.parameters->BindParameterExpression(expr); diff --git a/test/sql/prepared/prepare_maintain_types.test b/test/sql/prepared/prepare_maintain_types.test new file mode 100644 index 000000000000..c3174f04d256 --- /dev/null +++ b/test/sql/prepared/prepare_maintain_types.test @@ -0,0 +1,19 @@ +# name: test/sql/prepared/prepare_maintain_types.test +# description: Maintain PREPARE parameter types +# group: [prepared] + +statement ok +PRAGMA enable_verification + +statement ok +prepare v1 as select cast(111 as short) * $1; + +query I +execute v1(1665::BIGINT); +---- +184815 + +statement error +execute v1(1665::SHORT); +---- +Overflow From 2d1ff9e87be3528085e9d1e41157dd4fdf0afed7 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Thu, 27 Jun 2024 11:36:04 +0200 Subject: [PATCH 2/6] Add C API prepared statement test --- test/api/capi/test_capi_prepared.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/test/api/capi/test_capi_prepared.cpp b/test/api/capi/test_capi_prepared.cpp index 7ee8a6c15098..7294616932b0 100644 --- a/test/api/capi/test_capi_prepared.cpp +++ b/test/api/capi/test_capi_prepared.cpp @@ -385,6 +385,30 @@ TEST_CASE("Test prepared statements with named parameters in C API", "[capi]") { duckdb_destroy_prepare(&stmt); } +TEST_CASE("Maintain prepared statement types", "[capi]") { + CAPITester tester; + duckdb::unique_ptr result; + duckdb_result res; + duckdb_prepared_statement stmt = nullptr; + duckdb_state status; + + // open the database in in-memory mode + REQUIRE(tester.OpenDatabase(nullptr)); + + status = duckdb_prepare(tester.connection, "select cast(111 as short) * $1", &stmt); + REQUIRE(status == DuckDBSuccess); + REQUIRE(stmt != nullptr); + + status = duckdb_bind_int64(stmt, 1, 1665); + REQUIRE(status == DuckDBSuccess); + + status = duckdb_execute_prepared(stmt, &res); + REQUIRE(status == DuckDBSuccess); + REQUIRE(duckdb_value_int64(&res, 0, 0) == 184815); + duckdb_destroy_result(&res); + duckdb_destroy_prepare(&stmt); +} + TEST_CASE("Prepared streaming result", "[capi]") { CAPITester tester; From 45b82ebfc7ede5d3627096c66511611402cc2344 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Thu, 27 Jun 2024 17:53:15 +0200 Subject: [PATCH 3/6] Move all prepared statement parameters to BoundParameterData instead of Value, and use the return_type to keep track of whether or not a literal has been supplied as a parameter --- .../duckdb/main/capi/capi_internal.hpp | 3 +- src/include/duckdb/main/client_context.hpp | 30 ++++++++++--------- .../duckdb/main/prepared_statement.hpp | 5 ++-- .../duckdb/main/prepared_statement_data.hpp | 4 +-- .../expression/bound_parameter_data.hpp | 2 ++ src/main/capi/prepared-c.cpp | 4 +-- src/main/client_context.cpp | 10 ++++--- src/main/prepared_statement.cpp | 8 ++--- src/main/prepared_statement_data.cpp | 11 +++---- .../expression/bind_parameter_expression.cpp | 7 +++-- src/planner/binder/statement/bind_execute.cpp | 30 ++++++++++++++----- 11 files changed, 71 insertions(+), 43 deletions(-) diff --git a/src/include/duckdb/main/capi/capi_internal.hpp b/src/include/duckdb/main/capi/capi_internal.hpp index 0f9a9c213ad6..dc75ad67c8bd 100644 --- a/src/include/duckdb/main/capi/capi_internal.hpp +++ b/src/include/duckdb/main/capi/capi_internal.hpp @@ -15,6 +15,7 @@ #include "duckdb/main/appender.hpp" #include "duckdb/common/case_insensitive_map.hpp" #include "duckdb/main/client_context.hpp" +#include "duckdb/planner/expression/bound_parameter_data.hpp" #include #include @@ -33,7 +34,7 @@ struct DatabaseData { struct PreparedStatementWrapper { //! Map of name -> values - case_insensitive_map_t values; + case_insensitive_map_t values; unique_ptr statement; }; diff --git a/src/include/duckdb/main/client_context.hpp b/src/include/duckdb/main/client_context.hpp index 353bd53322bf..97fbe2ce1bcd 100644 --- a/src/include/duckdb/main/client_context.hpp +++ b/src/include/duckdb/main/client_context.hpp @@ -10,24 +10,25 @@ #include "duckdb/catalog/catalog_entry/schema_catalog_entry.hpp" #include "duckdb/catalog/catalog_set.hpp" -#include "duckdb/common/enums/pending_execution_result.hpp" +#include "duckdb/common/atomic.hpp" #include "duckdb/common/deque.hpp" +#include "duckdb/common/enums/pending_execution_result.hpp" +#include "duckdb/common/enums/prepared_statement_mode.hpp" +#include "duckdb/common/error_data.hpp" #include "duckdb/common/pair.hpp" #include "duckdb/common/unordered_set.hpp" #include "duckdb/common/winapi.hpp" +#include "duckdb/main/client_config.hpp" +#include "duckdb/main/client_context_state.hpp" +#include "duckdb/main/client_properties.hpp" +#include "duckdb/main/external_dependencies.hpp" +#include "duckdb/main/pending_query_result.hpp" #include "duckdb/main/prepared_statement.hpp" +#include "duckdb/main/settings.hpp" #include "duckdb/main/stream_query_result.hpp" #include "duckdb/main/table_description.hpp" #include "duckdb/transaction/transaction_context.hpp" -#include "duckdb/main/pending_query_result.hpp" -#include "duckdb/common/atomic.hpp" -#include "duckdb/main/client_config.hpp" -#include "duckdb/main/external_dependencies.hpp" -#include "duckdb/common/error_data.hpp" -#include "duckdb/common/enums/prepared_statement_mode.hpp" -#include "duckdb/main/client_properties.hpp" -#include "duckdb/main/client_context_state.hpp" -#include "duckdb/main/settings.hpp" +#include "duckdb/planner/expression/bound_parameter_data.hpp" namespace duckdb { class Appender; @@ -52,7 +53,7 @@ class ClientContextState; struct PendingQueryParameters { //! Prepared statement parameters (if any) - optional_ptr> parameters; + optional_ptr> parameters; //! Whether or not a stream result should be allowed bool allow_stream_result = false; }; @@ -142,7 +143,8 @@ class ClientContext : public enable_shared_from_this { //! It is possible that the prepared statement will be re-bound. This will generally happen if the catalog is //! modified in between the prepared statement being bound and the prepared statement being run. DUCKDB_API unique_ptr Execute(const string &query, shared_ptr &prepared, - case_insensitive_map_t &values, bool allow_stream_result = true); + case_insensitive_map_t &values, + bool allow_stream_result = true); DUCKDB_API unique_ptr Execute(const string &query, shared_ptr &prepared, const PendingQueryParameters ¶meters); @@ -227,7 +229,7 @@ class ClientContext : public enable_shared_from_this { //! Internally prepare a SQL statement. Caller must hold the context_lock. shared_ptr CreatePreparedStatement(ClientContextLock &lock, const string &query, unique_ptr statement, - optional_ptr> values = nullptr, + optional_ptr> values = nullptr, PreparedStatementMode mode = PreparedStatementMode::PREPARE_ONLY); unique_ptr PendingStatementInternal(ClientContextLock &lock, const string &query, unique_ptr statement, @@ -268,7 +270,7 @@ class ClientContext : public enable_shared_from_this { shared_ptr CreatePreparedStatementInternal(ClientContextLock &lock, const string &query, unique_ptr statement, - optional_ptr> values); + optional_ptr> values); private: //! Lock on using the ClientContext in parallel diff --git a/src/include/duckdb/main/prepared_statement.hpp b/src/include/duckdb/main/prepared_statement.hpp index f1368ca42ba4..9e55239b3299 100644 --- a/src/include/duckdb/main/prepared_statement.hpp +++ b/src/include/duckdb/main/prepared_statement.hpp @@ -13,6 +13,7 @@ #include "duckdb/main/pending_query_result.hpp" #include "duckdb/common/error_data.hpp" #include "duckdb/common/case_insensitive_map.hpp" +#include "duckdb/planner/expression/bound_parameter_data.hpp" namespace duckdb { class ClientContext; @@ -76,14 +77,14 @@ class PreparedStatement { DUCKDB_API unique_ptr PendingQuery(vector &values, bool allow_stream_result = true); //! Create a pending query result of the prepared statement with the given set named arguments - DUCKDB_API unique_ptr PendingQuery(case_insensitive_map_t &named_values, + DUCKDB_API unique_ptr PendingQuery(case_insensitive_map_t &named_values, bool allow_stream_result = true); //! Execute the prepared statement with the given set of values DUCKDB_API unique_ptr Execute(vector &values, bool allow_stream_result = true); //! Execute the prepared statement with the given set of named+unnamed values - DUCKDB_API unique_ptr Execute(case_insensitive_map_t &named_values, + DUCKDB_API unique_ptr Execute(case_insensitive_map_t &named_values, bool allow_stream_result = true); //! Execute the prepared statement with the given set of arguments diff --git a/src/include/duckdb/main/prepared_statement_data.hpp b/src/include/duckdb/main/prepared_statement_data.hpp index 684f2d53061d..8ebcf0b3b9d6 100644 --- a/src/include/duckdb/main/prepared_statement_data.hpp +++ b/src/include/duckdb/main/prepared_statement_data.hpp @@ -52,9 +52,9 @@ class PreparedStatementData { public: void CheckParameterCount(idx_t parameter_count); //! Whether or not the prepared statement data requires the query to rebound for the given parameters - bool RequireRebind(ClientContext &context, optional_ptr> values); + bool RequireRebind(ClientContext &context, optional_ptr> values); //! Bind a set of values to the prepared statement data - DUCKDB_API void Bind(case_insensitive_map_t values); + DUCKDB_API void Bind(case_insensitive_map_t values); //! Get the expected SQL Type of the bound parameter DUCKDB_API LogicalType GetType(const string &identifier); //! Try to get the expected SQL Type of the bound parameter diff --git a/src/include/duckdb/planner/expression/bound_parameter_data.hpp b/src/include/duckdb/planner/expression/bound_parameter_data.hpp index b1cac717d514..181751c1e58a 100644 --- a/src/include/duckdb/planner/expression/bound_parameter_data.hpp +++ b/src/include/duckdb/planner/expression/bound_parameter_data.hpp @@ -19,6 +19,8 @@ struct BoundParameterData { } explicit BoundParameterData(Value val) : value(std::move(val)), return_type(value.type()) { } + BoundParameterData(Value val, LogicalType type_p) : value(std::move(val)), return_type(std::move(type_p)) { + } private: Value value; diff --git a/src/main/capi/prepared-c.cpp b/src/main/capi/prepared-c.cpp index 8ac414978f7e..0302f92af60f 100644 --- a/src/main/capi/prepared-c.cpp +++ b/src/main/capi/prepared-c.cpp @@ -135,7 +135,7 @@ duckdb_type duckdb_param_type(duckdb_prepared_statement prepared_statement, idx_ // See if this is the case and we still have a value registered for it auto it = wrapper->values.find(identifier); if (it != wrapper->values.end()) { - return ConvertCPPTypeToC(it->second.type()); + return ConvertCPPTypeToC(it->second.return_type.id()); } return DUCKDB_TYPE_INVALID; } @@ -162,7 +162,7 @@ duckdb_state duckdb_bind_value(duckdb_prepared_statement prepared_statement, idx return DuckDBError; } auto identifier = duckdb_parameter_name_internal(prepared_statement, param_idx); - wrapper->values[identifier] = *value; + wrapper->values[identifier] = duckdb::BoundParameterData(*value); return DuckDBSuccess; } diff --git a/src/main/client_context.cpp b/src/main/client_context.cpp index ebea69db9272..9f099dbef69c 100644 --- a/src/main/client_context.cpp +++ b/src/main/client_context.cpp @@ -310,7 +310,7 @@ static bool IsExplainAnalyze(SQLStatement *statement) { shared_ptr ClientContext::CreatePreparedStatementInternal(ClientContextLock &lock, const string &query, unique_ptr statement, - optional_ptr> values) { + optional_ptr> values) { StatementType statement_type = statement->type; auto result = make_shared_ptr(statement_type); @@ -369,7 +369,8 @@ ClientContext::CreatePreparedStatementInternal(ClientContextLock &lock, const st shared_ptr ClientContext::CreatePreparedStatement(ClientContextLock &lock, const string &query, unique_ptr statement, - optional_ptr> values, PreparedStatementMode mode) { + optional_ptr> values, + PreparedStatementMode mode) { // check if any client context state could request a rebind bool can_request_rebind = false; for (auto const &s : registered_state) { @@ -420,7 +421,7 @@ QueryProgress ClientContext::GetQueryProgress() { } void BindPreparedStatementParameters(PreparedStatementData &statement, const PendingQueryParameters ¶meters) { - case_insensitive_map_t owned_values; + case_insensitive_map_t owned_values; if (parameters.parameters) { auto ¶ms = *parameters.parameters; for (auto &val : params) { @@ -712,7 +713,8 @@ unique_ptr ClientContext::Execute(const string &query, shared_ptr

ClientContext::Execute(const string &query, shared_ptr &prepared, - case_insensitive_map_t &values, bool allow_stream_result) { + case_insensitive_map_t &values, + bool allow_stream_result) { PendingQueryParameters parameters; parameters.parameters = &values; parameters.allow_stream_result = allow_stream_result; diff --git a/src/main/prepared_statement.cpp b/src/main/prepared_statement.cpp index df955171dd46..40471d677cfe 100644 --- a/src/main/prepared_statement.cpp +++ b/src/main/prepared_statement.cpp @@ -68,7 +68,7 @@ case_insensitive_map_t PreparedStatement::GetExpectedParameterTypes return expected_types; } -unique_ptr PreparedStatement::Execute(case_insensitive_map_t &named_values, +unique_ptr PreparedStatement::Execute(case_insensitive_map_t &named_values, bool allow_stream_result) { auto pending = PendingQuery(named_values, allow_stream_result); if (pending->HasError()) { @@ -86,15 +86,15 @@ unique_ptr PreparedStatement::Execute(vector &values, bool a } unique_ptr PreparedStatement::PendingQuery(vector &values, bool allow_stream_result) { - case_insensitive_map_t named_values; + case_insensitive_map_t named_values; for (idx_t i = 0; i < values.size(); i++) { auto &val = values[i]; - named_values[std::to_string(i + 1)] = val; + named_values[std::to_string(i + 1)] = BoundParameterData(val); } return PendingQuery(named_values, allow_stream_result); } -unique_ptr PreparedStatement::PendingQuery(case_insensitive_map_t &named_values, +unique_ptr PreparedStatement::PendingQuery(case_insensitive_map_t &named_values, bool allow_stream_result) { if (!success) { auto exception = InvalidInputException("Attempting to execute an unsuccessfully prepared statement!"); diff --git a/src/main/prepared_statement_data.cpp b/src/main/prepared_statement_data.cpp index a6eb943a6ea3..c2c43d00cbb6 100644 --- a/src/main/prepared_statement_data.cpp +++ b/src/main/prepared_statement_data.cpp @@ -29,7 +29,8 @@ void StartTransactionInCatalog(ClientContext &context, const string &catalog_nam Transaction::Get(context, *database); } -bool PreparedStatementData::RequireRebind(ClientContext &context, optional_ptr> values) { +bool PreparedStatementData::RequireRebind(ClientContext &context, + optional_ptr> values) { idx_t count = values ? values->size() : 0; CheckParameterCount(count); if (!unbound_statement) { @@ -49,7 +50,7 @@ bool PreparedStatementData::RequireRebind(ClientContext &context, optional_ptrend()) { break; } - if (lookup->second.type() != it.second->return_type) { + if (lookup->second.GetValue().type() != it.second->return_type) { return true; } } @@ -68,7 +69,7 @@ bool PreparedStatementData::RequireRebind(ClientContext &context, optional_ptr values) { +void PreparedStatementData::Bind(case_insensitive_map_t values) { // set parameters D_ASSERT(!unbound_statement || unbound_statement->n_param == properties.parameter_count); CheckParameterCount(values.size()); @@ -81,13 +82,13 @@ void PreparedStatementData::Bind(case_insensitive_map_t values) { throw BinderException("Could not find parameter with identifier %s", identifier); } D_ASSERT(it.second); - auto &value = lookup->second; + auto value = lookup->second.GetValue(); if (!value.DefaultTryCastAs(it.second->return_type)) { throw BinderException( "Type mismatch for binding parameter with identifier %s, expected type %s but got type %s", identifier, it.second->return_type.ToString().c_str(), value.type().ToString().c_str()); } - it.second->SetValue(value); + it.second->SetValue(std::move(value)); } } diff --git a/src/planner/binder/expression/bind_parameter_expression.cpp b/src/planner/binder/expression/bind_parameter_expression.cpp index 04e24211d452..678faf269300 100644 --- a/src/planner/binder/expression/bind_parameter_expression.cpp +++ b/src/planner/binder/expression/bind_parameter_expression.cpp @@ -21,10 +21,13 @@ BindResult ExpressionBinder::BindExpression(ParameterExpression &expr, idx_t dep // it has! emit a constant directly auto &data = param_data_it->second; auto return_type = binder.parameters->GetReturnType(parameter_id); + bool is_literal = + return_type.id() == LogicalTypeId::INTEGER_LITERAL || return_type.id() == LogicalTypeId::STRING_LITERAL; auto constant = make_uniq(data.GetValue()); constant->alias = expr.alias; - constant->return_type = return_type; - + if (is_literal) { + return BindResult(std::move(constant)); + } auto cast = BoundCastExpression::AddCastToType(context, std::move(constant), return_type); return BindResult(std::move(cast)); } diff --git a/src/planner/binder/statement/bind_execute.cpp b/src/planner/binder/statement/bind_execute.cpp index b8c43c711f78..9342f1e9a806 100644 --- a/src/planner/binder/statement/bind_execute.cpp +++ b/src/planner/binder/statement/bind_execute.cpp @@ -7,6 +7,7 @@ #include "duckdb/main/client_data.hpp" #include "duckdb/execution/expression_executor.hpp" #include "duckdb/common/case_insensitive_map.hpp" +#include "duckdb/planner/expression/bound_constant_expression.hpp" namespace duckdb { @@ -30,24 +31,39 @@ BoundStatement Binder::Bind(ExecuteStatement &stmt) { auto &mapped_named_values = stmt.named_values; // bind any supplied parameters - case_insensitive_map_t bind_values; + case_insensitive_map_t bind_values; auto constant_binder = Binder::CreateBinder(context); constant_binder->SetCanContainNulls(true); for (auto &pair : mapped_named_values) { + bool is_literal = pair.second->type == ExpressionType::VALUE_CONSTANT; + ConstantBinder cbinder(*constant_binder, context, "EXECUTE statement"); auto bound_expr = cbinder.Bind(pair.second); - - Value value = ExpressionExecutor::EvaluateScalar(context, *bound_expr, true); - bind_values[pair.first] = std::move(value); + BoundParameterData parameter_data; + if (is_literal) { + auto &constant = bound_expr->Cast(); + LogicalType return_type; + if (constant.return_type == LogicalTypeId::VARCHAR && + StringType::GetCollation(constant.return_type).empty()) { + return_type = LogicalTypeId::STRING_LITERAL; + } else if (constant.return_type.IsIntegral()) { + return_type = LogicalType::INTEGER_LITERAL(constant.value); + } else { + return_type = constant.value.type(); + } + parameter_data = BoundParameterData(std::move(constant.value), std::move(return_type)); + } else { + auto value = ExpressionExecutor::EvaluateScalar(context, *bound_expr, true); + parameter_data = BoundParameterData(std::move(value)); + } + bind_values[pair.first] = std::move(parameter_data); } unique_ptr rebound_plan; if (prepared->RequireRebind(context, &bind_values)) { // catalog was modified or statement does not have clear types: rebind the statement before running the execute Planner prepared_planner(context); - for (auto &pair : bind_values) { - prepared_planner.parameter_data.emplace(std::make_pair(pair.first, BoundParameterData(pair.second))); - } + prepared_planner.parameter_data = bind_values; prepared = prepared_planner.PrepareSQLStatement(entry->second->unbound_statement->Copy()); rebound_plan = std::move(prepared_planner.plan); D_ASSERT(prepared->properties.bound_all_parameters); From 4be3e2196a2ff7f15f61caaf04412e7c8c7b81d0 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Fri, 28 Jun 2024 09:13:33 +0200 Subject: [PATCH 4/6] Fixes for Python package --- .../duckdb_python/pyconnection/pyconnection.hpp | 3 ++- tools/pythonpkg/src/pyconnection.cpp | 12 ++++++------ 2 files changed, 8 insertions(+), 7 deletions(-) diff --git a/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp b/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp index b3115ac9a67a..9a5d3b5b82ae 100644 --- a/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp +++ b/tools/pythonpkg/src/include/duckdb_python/pyconnection/pyconnection.hpp @@ -26,6 +26,7 @@ #include "duckdb/common/shared_ptr.hpp" namespace duckdb { +struct BoundParameterData; enum class PythonEnvironmentType { NORMAL, INTERACTIVE, JUPYTER }; @@ -207,7 +208,7 @@ struct DuckDBPyConnection : public enable_shared_from_this { static shared_ptr Connect(const string &database, bool read_only, const py::dict &config); static vector TransformPythonParamList(const py::handle ¶ms); - static case_insensitive_map_t TransformPythonParamDict(const py::dict ¶ms); + static case_insensitive_map_t TransformPythonParamDict(const py::dict ¶ms); void RegisterFilesystem(AbstractFileSystem filesystem); void UnregisterFilesystem(const py::str &name); diff --git a/tools/pythonpkg/src/pyconnection.cpp b/tools/pythonpkg/src/pyconnection.cpp index b4fd95c1d425..587abe513fdb 100644 --- a/tools/pythonpkg/src/pyconnection.cpp +++ b/tools/pythonpkg/src/pyconnection.cpp @@ -524,8 +524,8 @@ py::list TransformNamedParameters(const case_insensitive_map_t &named_par return new_params; } -case_insensitive_map_t TransformPreparedParameters(PreparedStatement &prep, const py::object ¶ms) { - case_insensitive_map_t named_values; +case_insensitive_map_t TransformPreparedParameters(PreparedStatement &prep, const py::object ¶ms) { + case_insensitive_map_t named_values; if (py::is_list_like(params)) { if (prep.n_param != py::len(params)) { throw InvalidInputException("Prepared statement needs %d parameters, %d given", prep.n_param, @@ -535,7 +535,7 @@ case_insensitive_map_t TransformPreparedParameters(PreparedStatement &pre for (idx_t i = 0; i < unnamed_values.size(); i++) { auto &value = unnamed_values[i]; auto identifier = std::to_string(i + 1); - named_values[identifier] = std::move(value); + named_values[identifier] = BoundParameterData(std::move(value)); } } else if (py::is_dict_like(params)) { auto dict = py::cast(params); @@ -1616,13 +1616,13 @@ vector DuckDBPyConnection::TransformPythonParamList(const py::handle &par return args; } -case_insensitive_map_t DuckDBPyConnection::TransformPythonParamDict(const py::dict ¶ms) { - case_insensitive_map_t args; +case_insensitive_map_t DuckDBPyConnection::TransformPythonParamDict(const py::dict ¶ms) { + case_insensitive_map_t args; for (auto pair : params) { auto &key = pair.first; auto &value = pair.second; - args[std::string(py::str(key))] = TransformPythonValue(value, LogicalType::UNKNOWN, false); + args[std::string(py::str(key))] = BoundParameterData(TransformPythonValue(value, LogicalType::UNKNOWN, false)); } return args; } From 28674d30dcd6d01c9dbd0bd85e8e25dd69cdfa11 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Fri, 28 Jun 2024 09:18:38 +0200 Subject: [PATCH 5/6] Format fix --- tools/pythonpkg/src/pyconnection.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tools/pythonpkg/src/pyconnection.cpp b/tools/pythonpkg/src/pyconnection.cpp index 587abe513fdb..deb881f9f275 100644 --- a/tools/pythonpkg/src/pyconnection.cpp +++ b/tools/pythonpkg/src/pyconnection.cpp @@ -524,7 +524,8 @@ py::list TransformNamedParameters(const case_insensitive_map_t &named_par return new_params; } -case_insensitive_map_t TransformPreparedParameters(PreparedStatement &prep, const py::object ¶ms) { +case_insensitive_map_t TransformPreparedParameters(PreparedStatement &prep, + const py::object ¶ms) { case_insensitive_map_t named_values; if (py::is_list_like(params)) { if (prep.n_param != py::len(params)) { From e12670b8212c3d327beef7da6b7a4baaddd12d98 Mon Sep 17 00:00:00 2001 From: Mark Raasveldt Date: Fri, 28 Jun 2024 13:11:55 +0200 Subject: [PATCH 6/6] More tests --- test/sql/prepared/prepare_maintain_types.test | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/test/sql/prepared/prepare_maintain_types.test b/test/sql/prepared/prepare_maintain_types.test index c3174f04d256..2a8df4f965e2 100644 --- a/test/sql/prepared/prepare_maintain_types.test +++ b/test/sql/prepared/prepare_maintain_types.test @@ -8,6 +8,23 @@ PRAGMA enable_verification statement ok prepare v1 as select cast(111 as short) * $1; +# literals adapt to the type +statement error +execute v1(1665); +---- +Overflow + +statement error +execute v1('1665'); +---- +Overflow + +# explicit casts force the type to be used +statement error +execute v1('1665'::VARCHAR); +---- +No function matches the given name and argument types + query I execute v1(1665::BIGINT); ----