Merge pull request #302 from traceon/bind-using-descriptors

Use descriptors for column binding information

README.md

@@ -130,7 +130,7 @@ The general requirements for building the driver from sources are as follows:
 - C++17 and C11 capable compiler toolchain:
   - Clang 4 and later
   - GCC 7 and later
-  - Xcode 10 and later
+  - Xcode 10 and later (on macOS 10.14 and later)
   - Microsoft Visual Studio 2017 and later
 
 Additional requirements exist for each platform, which also depend on whether packaging and/or testing is performed.
@@ -363,7 +363,7 @@ cmake --open .
 
 #### Build-time dependencies <!-- omit in toc -->
 
-You will need Xcode 10 or later and Command Line Tools to be installed, as well as [Homebrew](https://brew.sh/).
+You will need macOS 10.14 or later, Xcode 10 or later with Command Line Tools installed, as well as up-to-date [Homebrew](https://brew.sh/) available in the system.
 
 #### Build-time dependencies: iODBC <!-- omit in toc -->
 

driver/api/impl/impl.h

@@ -94,6 +94,15 @@ namespace impl {
         SQLSMALLINT * out_message_size
     ) noexcept;
 
+    SQLRETURN BindCol(
+        SQLHSTMT       StatementHandle,
+        SQLUSMALLINT   ColumnNumber,
+        SQLSMALLINT    TargetType,
+        SQLPOINTER     TargetValuePtr,
+        SQLLEN         BufferLength,
+        SQLLEN *       StrLen_or_Ind
+    ) noexcept;
+
     SQLRETURN BindParameter(
         SQLHSTMT        handle,
         SQLUSMALLINT    parameter_number,
@@ -177,15 +186,22 @@ namespace impl {
     ) noexcept;
 
     SQLRETURN GetData(
-        Statement & statement,
-        SQLUSMALLINT column_or_param_number,
-        const BindingInfo & binding_info
-    );
+        SQLHSTMT       StatementHandle,
+        SQLUSMALLINT   Col_or_Param_Num,
+        SQLSMALLINT    TargetType,
+        SQLPOINTER     TargetValuePtr,
+        SQLLEN         BufferLength,
+        SQLLEN *       StrLen_or_IndPtr
+    ) noexcept;
+
+    SQLRETURN Fetch(
+        SQLHSTMT       StatementHandle
+    ) noexcept;
 
     SQLRETURN FetchScroll(
-        Statement & statement,
-        SQLSMALLINT orientation,
-        SQLLEN offset
-    );
+        SQLHSTMT      StatementHandle,
+        SQLSMALLINT   FetchOrientation,
+        SQLLEN        FetchOffset
+    ) noexcept;
 
 } // namespace impl

driver/api/odbc.cpp

@@ -700,92 +700,64 @@ SQLRETURN SQL_API EXPORTED_FUNCTION_MAYBE_W(SQLDescribeCol)(HSTMT statement_hand
     return CALL_WITH_TYPED_HANDLE(SQL_HANDLE_STMT, statement_handle, func);
 }
 
-SQLRETURN SQL_API EXPORTED_FUNCTION(SQLFetch)(HSTMT statement_handle) {
-    auto func = [&] (Statement & statement) {
-        return impl::FetchScroll(statement, SQL_FETCH_NEXT, 0);
-    };
-
-    return CALL_WITH_TYPED_HANDLE(SQL_HANDLE_STMT, statement_handle, func);
+SQLRETURN SQL_API EXPORTED_FUNCTION(SQLFetch)(
+    SQLHSTMT     StatementHandle
+) {
+    LOG(__FUNCTION__);
+    return impl::Fetch(
+        StatementHandle
+    );
 }
 
-SQLRETURN SQL_API EXPORTED_FUNCTION(SQLFetchScroll)(HSTMT statement_handle, SQLSMALLINT orientation, SQLLEN offset) {
-    auto func = [&] (Statement & statement) {
-        return impl::FetchScroll(statement, orientation, offset);
-    };
-
-    return CALL_WITH_TYPED_HANDLE(SQL_HANDLE_STMT, statement_handle, func);
+SQLRETURN SQL_API EXPORTED_FUNCTION(SQLFetchScroll)(
+    SQLHSTMT      StatementHandle,
+    SQLSMALLINT   FetchOrientation,
+    SQLLEN        FetchOffset
+) {
+    LOG(__FUNCTION__);
+    return impl::FetchScroll(
+        StatementHandle,
+        FetchOrientation,
+        FetchOffset
+    );
 }
 
 SQLRETURN SQL_API EXPORTED_FUNCTION(SQLGetData)(
-    HSTMT statement_handle,
-    SQLUSMALLINT column_or_param_number,
-    SQLSMALLINT target_type,
-    PTR out_value,
-    SQLLEN out_value_max_size,
-    SQLLEN * out_value_size_or_indicator
+    SQLHSTMT       StatementHandle,
+    SQLUSMALLINT   Col_or_Param_Num,
+    SQLSMALLINT    TargetType,
+    SQLPOINTER     TargetValuePtr,
+    SQLLEN         BufferLength,
+    SQLLEN *       StrLen_or_IndPtr
 ) {
-    auto func = [&] (Statement & statement) {
-        BindingInfo binding_info;
-        binding_info.c_type = target_type;
-        binding_info.value = out_value;
-        binding_info.value_max_size = out_value_max_size;
-        binding_info.value_size = out_value_size_or_indicator;
-        binding_info.indicator = out_value_size_or_indicator;
-
-        return impl::GetData(
-            statement,
-            column_or_param_number,
-            binding_info
-        );
-    };
-
-    return CALL_WITH_TYPED_HANDLE(SQL_HANDLE_STMT, statement_handle, func);
+    LOG(__FUNCTION__);
+    return impl::GetData(
+        StatementHandle,
+        Col_or_Param_Num,
+        TargetType,
+        TargetValuePtr,
+        BufferLength,
+        StrLen_or_IndPtr
+    );
 }
 
 SQLRETURN SQL_API EXPORTED_FUNCTION(SQLBindCol)(
-    HSTMT statement_handle,
-    SQLUSMALLINT column_number,
-    SQLSMALLINT target_type,
-    PTR out_value,
-    SQLLEN out_value_max_size,
-    SQLLEN * out_value_size_or_indicator
+    SQLHSTMT       StatementHandle,
+    SQLUSMALLINT   ColumnNumber,
+    SQLSMALLINT    TargetType,
+    SQLPOINTER     TargetValuePtr,
+    SQLLEN         BufferLength,
+    SQLLEN *       StrLen_or_Ind
 ) {
-    auto func = [&] (Statement & statement) {
-        if (out_value_max_size < 0)
-            throw SqlException("Invalid string or buffer length", "HY090");
-
-        if (!statement.hasResultSet())
-            throw SqlException("Column info is not available", "07005");
-
-        auto & result_set = statement.getResultSet();
-
-        if (column_number < 1)
-            throw SqlException("Invalid descriptor index", "07009");
-
-        // Unbinding column
-        if (out_value_size_or_indicator == nullptr) {
-            statement.bindings.erase(column_number);
-            return SQL_SUCCESS;
-        }
-
-        const auto column_idx = column_number - 1;
-
-        if (target_type == SQL_C_DEFAULT)
-            target_type = statement.getTypeInfo(result_set.getColumnInfo(column_idx).type_without_parameters).sql_type;
-
-        BindingInfo binding;
-        binding.c_type = target_type;
-        binding.value = out_value;
-        binding.value_max_size = out_value_max_size;
-        binding.value_size = out_value_size_or_indicator;
-        binding.indicator = out_value_size_or_indicator;
-
-        statement.bindings[column_number] = binding;
-
-        return SQL_SUCCESS;
-    };
-
-    return CALL_WITH_TYPED_HANDLE(SQL_HANDLE_STMT, statement_handle, func);
+    LOG(__FUNCTION__);
+    return impl::BindCol(
+        StatementHandle,
+        ColumnNumber,
+        TargetType,
+        TargetValuePtr,
+        BufferLength,
+        StrLen_or_Ind
+    );
 }
 
 SQLRETURN SQL_API EXPORTED_FUNCTION(SQLRowCount)(HSTMT statement_handle, SQLLEN * out_row_count) {

driver/descriptor.cpp

@@ -105,7 +105,7 @@ void DescriptorRecord::onAttrChange(int attr) {
             break;
         }
         case SQL_DESC_DATA_PTR: {
-            if (getAttrAs<SQLPOINTER>(SQL_DESC_DATA_PTR, nullptr))
+            if (getAttrAs<SQLPOINTER>(SQL_DESC_DATA_PTR, 0))
                 consistencyCheck();
             break;
         }
@@ -222,3 +222,7 @@ DescriptorRecord & Descriptor::getRecord(std::size_t num, SQLINTEGER current_rol
 
     return records[num];
 }
+
+const std::vector<DescriptorRecord> & Descriptor::getRecordContainer() const {
+    return records;
+}

driver/descriptor.h

@@ -35,6 +35,8 @@ class Descriptor
     std::size_t getRecordCount() const;
     DescriptorRecord& getRecord(std::size_t num, SQLINTEGER current_role);
 
+    const std::vector<DescriptorRecord> & getRecordContainer() const;
+
 private:
     std::vector<DescriptorRecord> records;
 };

driver/statement.cpp

@@ -30,6 +30,8 @@ const TypeInfo & Statement::getTypeInfo(const std::string & type_name, const std
 
 void Statement::prepareQuery(const std::string & q) {
     closeCursor();
+
+    is_prepared = false;
     query = q;
     processEscapeSequences();
     extractParametersinfo();
@@ -57,7 +59,7 @@ void Statement::executeQuery(std::unique_ptr<ResultMutator> && mutator) {
     if (param_set_processed_ptr)
         *param_set_processed_ptr = 0;
 
-    next_param_set = 0;
+    next_param_set_idx = 0;
     requestNextPackOfResultSets(std::move(mutator));
     is_executed = true;
 }
@@ -66,7 +68,7 @@ void Statement::requestNextPackOfResultSets(std::unique_ptr<ResultMutator> && mu
     result_reader.reset();
 
     const auto param_set_array_size = getEffectiveDescriptor(SQL_ATTR_APP_PARAM_DESC).getAttrAs<SQLULEN>(SQL_DESC_ARRAY_SIZE, 1);
-    if (next_param_set >= param_set_array_size)
+    if (next_param_set_idx >= param_set_array_size)
         return;
 
     getDiagHeader().setAttr(SQL_DIAG_ROW_COUNT, 0);
@@ -103,7 +105,7 @@ void Statement::requestNextPackOfResultSets(std::unique_ptr<ResultMutator> && mu
     if (!database_set)
         uri.addQueryParameter("database", connection.database);
 
-    const auto param_bindings = getParamsBindingInfo(next_param_set);
+    const auto param_bindings = getParamsBindingInfo(next_param_set_idx);
 
     for (std::size_t i = 0; i < parameters.size(); ++i) {
         std::string value;
@@ -132,7 +134,7 @@ void Statement::requestNextPackOfResultSets(std::unique_ptr<ResultMutator> && mu
     // TODO: set this only after this single query is fully fetched (when output parameter support is added)
     auto * param_set_processed_ptr = getEffectiveDescriptor(SQL_ATTR_IMP_PARAM_DESC).getAttrAs<SQLULEN *>(SQL_DESC_ROWS_PROCESSED_PTR, 0);
     if (param_set_processed_ptr)
-        *param_set_processed_ptr = next_param_set;
+        *param_set_processed_ptr = next_param_set_idx;
 
     Poco::Net::HTTPRequest request;
     request.setMethod(Poco::Net::HTTPRequest::HTTP_POST);
@@ -190,7 +192,7 @@ void Statement::requestNextPackOfResultSets(std::unique_ptr<ResultMutator> && mu
     }
 
     result_reader = make_result_reader(response->get("X-ClickHouse-Format", connection.default_format), *in, std::move(mutator));
-    ++next_param_set;
+    ++next_param_set_idx;
 }
 
 void Statement::processEscapeSequences() {
@@ -205,7 +207,7 @@ void Statement::extractParametersinfo() {
     const auto apd_record_count = apd_desc.getRecordCount();
     auto ipd_record_count = ipd_desc.getRecordCount();
 
-    // Reset IPD records but preserve possible info set by SQLBindParameter.
+    // Reset IPD records but preserve those that may have been modified by SQLBindParameter and are still relevant.
     ipd_record_count = std::min(ipd_record_count, apd_record_count);
     ipd_desc.setAttr(SQL_DESC_COUNT, ipd_record_count);
 
@@ -295,8 +297,9 @@ void Statement::extractParametersinfo() {
         }
     }
 
-    ipd_record_count = std::max(ipd_record_count, parameters.size());
-    ipd_desc.setAttr(SQL_DESC_COUNT, ipd_record_count);
+    // Access the biggest record to [possibly] create all missing ones.
+    if (ipd_record_count < parameters.size())
+        ipd_desc.getRecord(parameters.size(), SQL_ATTR_IMP_PARAM_DESC);
 }
 
 std::string Statement::buildFinalQuery(const std::vector<ParamBindingInfo>& param_bindings) {
@@ -389,16 +392,11 @@ void Statement::closeCursor() {
     in = nullptr;
     response.reset();
 
-    parameters.clear();
-    query.clear();
     is_executed = false;
     is_forward_executed = false;
-    is_prepared = false;
 }
 
 void Statement::resetColBindings() {
-    bindings.clear();
-
     getEffectiveDescriptor(SQL_ATTR_APP_ROW_DESC).setAttr(SQL_DESC_COUNT, 0);
 }
 
@@ -437,27 +435,32 @@ std::vector<ParamBindingInfo> Statement::getParamsBindingInfo(std::size_t param_
     if (fully_bound_param_count > 0)
         param_bindings.reserve(fully_bound_param_count);
 
-    const auto single_set_struct_size = apd_desc.getAttrAs<SQLULEN>(SQL_DESC_BIND_TYPE, SQL_PARAM_BIND_BY_COLUMN);
+    auto * array_status_ptr = ipd_desc.getAttrAs<SQLUSMALLINT *>(SQL_DESC_ARRAY_STATUS_PTR, 0);
+
+    const auto bind_type = apd_desc.getAttrAs<SQLULEN>(SQL_DESC_BIND_TYPE, SQL_PARAM_BIND_TYPE_DEFAULT);
     const auto * bind_offset_ptr = apd_desc.getAttrAs<SQLULEN *>(SQL_DESC_BIND_OFFSET_PTR, 0);
     const auto bind_offset = (bind_offset_ptr ? *bind_offset_ptr : 0);
 
-    for (std::size_t i = 1; i <= fully_bound_param_count; ++i) {
-        ParamBindingInfo binding_info;
-
-        auto & apd_record = apd_desc.getRecord(i, SQL_ATTR_APP_PARAM_DESC);
-        auto & ipd_record = ipd_desc.getRecord(i, SQL_ATTR_IMP_PARAM_DESC);
+    for (std::size_t param_num = 1; param_num <= fully_bound_param_count; ++param_num) {
+        auto & apd_record = apd_desc.getRecord(param_num, SQL_ATTR_APP_PARAM_DESC);
+        auto & ipd_record = ipd_desc.getRecord(param_num, SQL_ATTR_IMP_PARAM_DESC);
 
         const auto * data_ptr = apd_record.getAttrAs<SQLPOINTER>(SQL_DESC_DATA_PTR, 0);
         const auto * sz_ptr = apd_record.getAttrAs<SQLLEN *>(SQL_DESC_OCTET_LENGTH_PTR, 0);
         const auto * ind_ptr = apd_record.getAttrAs<SQLLEN *>(SQL_DESC_INDICATOR_PTR, 0);
 
+        ParamBindingInfo binding_info;
         binding_info.io_type = ipd_record.getAttrAs<SQLSMALLINT>(SQL_DESC_PARAMETER_TYPE, SQL_PARAM_INPUT);
         binding_info.c_type = apd_record.getAttrAs<SQLSMALLINT>(SQL_DESC_CONCISE_TYPE, SQL_C_DEFAULT);
         binding_info.sql_type = ipd_record.getAttrAs<SQLSMALLINT>(SQL_DESC_CONCISE_TYPE, SQL_UNKNOWN_TYPE);
-        binding_info.value_max_size = ipd_record.getAttrAs<SQLULEN>(SQL_DESC_LENGTH, 0); // TODO: or SQL_DESC_OCTET_LENGTH ?
-        binding_info.value = (void *)(data_ptr ? ((char *)(data_ptr) + param_set_idx * single_set_struct_size + bind_offset) : 0);
-        binding_info.value_size = (SQLLEN *)(sz_ptr ? ((char *)(sz_ptr) + param_set_idx * sizeof(SQLLEN) + bind_offset) : 0);
-        binding_info.indicator = (SQLLEN *)(ind_ptr ? ((char *)(ind_ptr) + param_set_idx * sizeof(SQLLEN) + bind_offset) : 0);
+        binding_info.value_max_size = ipd_record.getAttrAs<SQLLEN>(SQL_DESC_OCTET_LENGTH, 0);
+
+        const auto next_value_ptr_increment = (bind_type == SQL_PARAM_BIND_BY_COLUMN ? binding_info.value_max_size : bind_type);
+        const auto next_sz_ind_ptr_increment = (bind_type == SQL_PARAM_BIND_BY_COLUMN ? sizeof(SQLLEN) : bind_type);
+
+        binding_info.value = (SQLPOINTER)(data_ptr ? ((char *)(data_ptr) + param_set_idx * next_value_ptr_increment + bind_offset) : 0);
+        binding_info.value_size = (SQLLEN *)(sz_ptr ? ((char *)(sz_ptr) + param_set_idx * next_sz_ind_ptr_increment + bind_offset) : 0);
+        binding_info.indicator = (SQLLEN *)(ind_ptr ? ((char *)(ind_ptr) + param_set_idx * next_sz_ind_ptr_increment + bind_offset) : 0);
 
         // TODO: always use SQL_NULLABLE as a default when https://github.com/ClickHouse/ClickHouse/issues/7488 is fixed.
         binding_info.is_nullable = (
@@ -474,6 +477,9 @@ std::vector<ParamBindingInfo> Statement::getParamsBindingInfo(std::size_t param_
         param_bindings.emplace_back(binding_info);
     }
 
+    if (array_status_ptr)
+        array_status_ptr[param_set_idx] = SQL_PARAM_SUCCESS; // TODO: elaborate?
+
     return param_bindings;
 }
 

driver/statement.h

@@ -105,9 +105,5 @@ class Statement
     std::unique_ptr<Poco::Net::HTTPResponse> response;
     std::istream* in = nullptr;
     std::unique_ptr<ResultReader> result_reader;
-    std::size_t next_param_set = 0;
-
-public:
-    // TODO: switch to using the corresponding descriptor attributes.
-    std::map<SQLUSMALLINT, BindingInfo> bindings;
+    std::size_t next_param_set_idx = 0;
 };

driver/test/CMakeLists.txt

@@ -67,6 +67,8 @@ function (declare_odbc_test_targets libname UNICODE)
         ${PROJECT_SOURCE_DIR}/driver/utils/conversion_std.h
         ${PROJECT_SOURCE_DIR}/driver/utils/conversion_icu.h
         misc_it.cpp
+        column_bindings_it.cpp
+        statement_parameter_bindings_it.cpp
         statement_parameters_it.cpp
         performance_it.cpp
     )