Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-26 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2383537715


##
cpp/src/arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.cc:
##
@@ -0,0 +1,140 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.h"
+
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+
+// The list of functions that can be converted from string to ODBC bitmasks is
+// based on Calcite's SqlJdbcFunctionCall class.
+
+namespace {
+static const std::unordered_map numeric_functions = {
+{"ABS", SQL_FN_NUM_ABS}, {"ACOS", SQL_FN_NUM_ACOS},
+{"ASIN", SQL_FN_NUM_ASIN},   {"ATAN", SQL_FN_NUM_ATAN},
+{"ATAN2", SQL_FN_NUM_ATAN2}, {"CEILING", SQL_FN_NUM_CEILING},
+{"COS", SQL_FN_NUM_ACOS},{"COT", SQL_FN_NUM_COT},
+{"DEGREES", SQL_FN_NUM_DEGREES}, {"EXP", SQL_FN_NUM_EXP},
+{"FLOOR", SQL_FN_NUM_FLOOR}, {"LOG", SQL_FN_NUM_LOG},
+{"LOG10", SQL_FN_NUM_LOG10}, {"MOD", SQL_FN_NUM_MOD},
+{"PI", SQL_FN_NUM_PI},   {"POWER", SQL_FN_NUM_POWER},
+{"RADIANS", SQL_FN_NUM_RADIANS}, {"RAND", SQL_FN_NUM_RAND},
+{"ROUND", SQL_FN_NUM_ROUND}, {"SIGN", SQL_FN_NUM_SIGN},
+{"SIN", SQL_FN_NUM_SIN}, {"SQRT", SQL_FN_NUM_SQRT},
+{"TAN", SQL_FN_NUM_TAN}, {"TRUNCATE", SQL_FN_NUM_TRUNCATE}};
+
+static const std::unordered_map system_functions = {
+{"DATABASE", SQL_FN_SYS_DBNAME},
+{"IFNULL", SQL_FN_SYS_IFNULL},
+{"USER", SQL_FN_SYS_USERNAME}};
+
+static const std::unordered_map datetime_functions = {
+{"CURDATE", SQL_FN_TD_CURDATE},
+{"CURTIME", SQL_FN_TD_CURTIME},
+{"DAYNAME", SQL_FN_TD_DAYNAME},
+{"DAYOFMONTH", SQL_FN_TD_DAYOFMONTH},
+{"DAYOFWEEK", SQL_FN_TD_DAYOFWEEK},
+{"DAYOFYEAR", SQL_FN_TD_DAYOFYEAR},
+{"HOUR", SQL_FN_TD_HOUR},
+{"MINUTE", SQL_FN_TD_MINUTE},
+{"MONTH", SQL_FN_TD_MONTH},
+{"MONTHNAME", SQL_FN_TD_MONTHNAME},
+{"NOW", SQL_FN_TD_NOW},
+{"QUARTER", SQL_FN_TD_QUARTER},
+{"SECOND", SQL_FN_TD_SECOND},
+{"TIMESTAMPADD", SQL_FN_TD_TIMESTAMPADD},
+{"TIMESTAMPDIFF", SQL_FN_TD_TIMESTAMPDIFF},
+{"WEEK", SQL_FN_TD_WEEK},
+{"YEAR", SQL_FN_TD_YEAR},
+// Additional functions in ODBC but not Calcite:
+{"CURRENT_DATE", SQL_FN_TD_CURRENT_DATE},
+{"CURRENT_TIME", SQL_FN_TD_CURRENT_TIME},
+{"CURRENT_TIMESTAMP", SQL_FN_TD_CURRENT_TIMESTAMP},
+{"EXTRACT", SQL_FN_TD_EXTRACT}};
+
+static const std::unordered_map string_functions = {
+{"ASCII", SQL_FN_STR_ASCII},
+{"CHAR", SQL_FN_STR_CHAR},
+{"CONCAT", SQL_FN_STR_CONCAT},
+{"DIFFERENCE", SQL_FN_STR_DIFFERENCE},
+{"INSERT", SQL_FN_STR_INSERT},
+{"LCASE", SQL_FN_STR_LCASE},
+{"LEFT", SQL_FN_STR_LEFT},
+{"LENGTH", SQL_FN_STR_LENGTH},
+{"LOCATE", SQL_FN_STR_LOCATE},
+{"LTRIM", SQL_FN_STR_LTRIM},
+{"REPEAT", SQL_FN_STR_REPEAT},
+{"REPLACE", SQL_FN_STR_REPLACE},
+{"RIGHT", SQL_FN_STR_RIGHT},
+{"RTRIM", SQL_FN_STR_RTRIM},
+{"SOUNDEX", SQL_FN_STR_SOUNDEX},
+{"SPACE", SQL_FN_STR_SPACE},
+{"SUBSTRING", SQL_FN_STR_SUBSTRING},
+{"UCASE", SQL_FN_STR_UCASE},
+// Additional functions in ODBC but not Calcite:

Review Comment:
   I think it isn't Dremio-specific. The Calcite's SqlJdbcFunctionCall class 
was referenced for this code. At the top of the file, it says
   ```
   // The list of functions that can be converted from string to ODBC bitmasks 
is
   // based on Calcite's SqlJdbcFunctionCall class.
   ```
   And I think this `Additional functions ...` comment is related to that. 
There exist a few system functions supported in ODBC, but not yet implemented 
on Calcite at the time the code is written. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-26 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2383276669


##
cpp/src/arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.cc:
##
@@ -0,0 +1,140 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.h"
+
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+
+// The list of functions that can be converted from string to ODBC bitmasks is
+// based on Calcite's SqlJdbcFunctionCall class.
+
+namespace {
+static const std::unordered_map numeric_functions = {
+{"ABS", SQL_FN_NUM_ABS}, {"ACOS", SQL_FN_NUM_ACOS},
+{"ASIN", SQL_FN_NUM_ASIN},   {"ATAN", SQL_FN_NUM_ATAN},
+{"ATAN2", SQL_FN_NUM_ATAN2}, {"CEILING", SQL_FN_NUM_CEILING},
+{"COS", SQL_FN_NUM_ACOS},{"COT", SQL_FN_NUM_COT},
+{"DEGREES", SQL_FN_NUM_DEGREES}, {"EXP", SQL_FN_NUM_EXP},
+{"FLOOR", SQL_FN_NUM_FLOOR}, {"LOG", SQL_FN_NUM_LOG},
+{"LOG10", SQL_FN_NUM_LOG10}, {"MOD", SQL_FN_NUM_MOD},
+{"PI", SQL_FN_NUM_PI},   {"POWER", SQL_FN_NUM_POWER},
+{"RADIANS", SQL_FN_NUM_RADIANS}, {"RAND", SQL_FN_NUM_RAND},
+{"ROUND", SQL_FN_NUM_ROUND}, {"SIGN", SQL_FN_NUM_SIGN},
+{"SIN", SQL_FN_NUM_SIN}, {"SQRT", SQL_FN_NUM_SQRT},
+{"TAN", SQL_FN_NUM_TAN}, {"TRUNCATE", SQL_FN_NUM_TRUNCATE}};
+
+static const std::unordered_map system_functions = {
+{"DATABASE", SQL_FN_SYS_DBNAME},
+{"IFNULL", SQL_FN_SYS_IFNULL},
+{"USER", SQL_FN_SYS_USERNAME}};
+
+static const std::unordered_map datetime_functions = {
+{"CURDATE", SQL_FN_TD_CURDATE},
+{"CURTIME", SQL_FN_TD_CURTIME},
+{"DAYNAME", SQL_FN_TD_DAYNAME},
+{"DAYOFMONTH", SQL_FN_TD_DAYOFMONTH},
+{"DAYOFWEEK", SQL_FN_TD_DAYOFWEEK},
+{"DAYOFYEAR", SQL_FN_TD_DAYOFYEAR},
+{"HOUR", SQL_FN_TD_HOUR},
+{"MINUTE", SQL_FN_TD_MINUTE},
+{"MONTH", SQL_FN_TD_MONTH},
+{"MONTHNAME", SQL_FN_TD_MONTHNAME},
+{"NOW", SQL_FN_TD_NOW},
+{"QUARTER", SQL_FN_TD_QUARTER},
+{"SECOND", SQL_FN_TD_SECOND},
+{"TIMESTAMPADD", SQL_FN_TD_TIMESTAMPADD},
+{"TIMESTAMPDIFF", SQL_FN_TD_TIMESTAMPDIFF},
+{"WEEK", SQL_FN_TD_WEEK},
+{"YEAR", SQL_FN_TD_YEAR},
+// Additional functions in ODBC but not Calcite:
+{"CURRENT_DATE", SQL_FN_TD_CURRENT_DATE},
+{"CURRENT_TIME", SQL_FN_TD_CURRENT_TIME},
+{"CURRENT_TIMESTAMP", SQL_FN_TD_CURRENT_TIMESTAMP},
+{"EXTRACT", SQL_FN_TD_EXTRACT}};
+
+static const std::unordered_map string_functions = {
+{"ASCII", SQL_FN_STR_ASCII},
+{"CHAR", SQL_FN_STR_CHAR},
+{"CONCAT", SQL_FN_STR_CONCAT},
+{"DIFFERENCE", SQL_FN_STR_DIFFERENCE},
+{"INSERT", SQL_FN_STR_INSERT},
+{"LCASE", SQL_FN_STR_LCASE},
+{"LEFT", SQL_FN_STR_LEFT},
+{"LENGTH", SQL_FN_STR_LENGTH},
+{"LOCATE", SQL_FN_STR_LOCATE},
+{"LTRIM", SQL_FN_STR_LTRIM},
+{"REPEAT", SQL_FN_STR_REPEAT},
+{"REPLACE", SQL_FN_STR_REPLACE},
+{"RIGHT", SQL_FN_STR_RIGHT},
+{"RTRIM", SQL_FN_STR_RTRIM},
+{"SOUNDEX", SQL_FN_STR_SOUNDEX},
+{"SPACE", SQL_FN_STR_SPACE},
+{"SUBSTRING", SQL_FN_STR_SUBSTRING},
+{"UCASE", SQL_FN_STR_UCASE},
+// Additional functions in ODBC but not Calcite:
+{"LOCATE_2", SQL_FN_STR_LOCATE_2},
+{"BIT_LENGTH", SQL_FN_STR_BIT_LENGTH},
+{"CHAR_LENGTH", SQL_FN_STR_CHAR_LENGTH},
+{"CHARACTER_LENGTH", SQL_FN_STR_CHARACTER_LENGTH},
+{"OCTET_LENGTH", SQL_FN_STR_OCTET_LENGTH},
+{"POSTION", SQL_FN_STR_POSITION},
+{"SOUNDEX", SQL_FN_STR_SOUNDEX}};
+}  // namespace
+
+void ReportSystemFunction(const std::string& function, uint32_t& 
current_sys_functions,
+  uint32_t& current_convert_functions) {
+  const auto& result = system_functions.find(function);
+  if (result != system_functions.end()) {
+current_sys_functions |= result->second;
+  } else if (function == "CONVERT") {
+// CAST and CONVERT are system functions from FlightSql/Calcite, but are

Review Comment:
   ```
   // CAST and CONVERT are system functions from FlightSql/Calcite, but are
   // 

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-25 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2380544662


##
cpp/src/arrow/flight/sql/odbc/flight_sql/get_info_cache.cc:
##
@@ -0,0 +1,1345 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/get_info_cache.h"
+
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+
+#include 
+#include 
+#include "arrow/array.h"
+#include "arrow/array/array_nested.h"
+#include "arrow/flight/sql/api.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/exceptions.h"
+#include "arrow/scalar.h"
+#include "arrow/type_fwd.h"
+
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_stream_chunk_buffer.h"
+#include "arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.h"
+#include "arrow/flight/sql/odbc/flight_sql/utils.h"
+
+// Aliases for entries in SqlInfoOptions::SqlInfo that are defined here
+// due to causing compilation errors conflicting with ODBC definitions.
+#define ARROW_SQL_IDENTIFIER_CASE 503
+#define ARROW_SQL_IDENTIFIER_QUOTE_CHAR 504
+#define ARROW_SQL_QUOTED_IDENTIFIER_CASE 505
+#define ARROW_SQL_KEYWORDS 508
+#define ARROW_SQL_NUMERIC_FUNCTIONS 509
+#define ARROW_SQL_STRING_FUNCTIONS 510
+#define ARROW_SQL_SYSTEM_FUNCTIONS 511
+#define ARROW_SQL_SCHEMA_TERM 529
+#define ARROW_SQL_PROCEDURE_TERM 530
+#define ARROW_SQL_CATALOG_TERM 531
+#define ARROW_SQL_MAX_COLUMNS_IN_GROUP_BY 544
+#define ARROW_SQL_MAX_COLUMNS_IN_INDEX 545
+#define ARROW_SQL_MAX_COLUMNS_IN_ORDER_BY 546
+#define ARROW_SQL_MAX_COLUMNS_IN_SELECT 547
+#define ARROW_SQL_MAX_COLUMNS_IN_TABLE 548
+#define ARROW_SQL_MAX_ROW_SIZE 555
+#define ARROW_SQL_MAX_TABLES_IN_SELECT 560
+
+#define ARROW_CONVERT_BIGINT 0
+#define ARROW_CONVERT_BINARY 1
+#define ARROW_CONVERT_BIT 2
+#define ARROW_CONVERT_CHAR 3
+#define ARROW_CONVERT_DATE 4
+#define ARROW_CONVERT_DECIMAL 5
+#define ARROW_CONVERT_FLOAT 6
+#define ARROW_CONVERT_INTEGER 7
+#define ARROW_CONVERT_INTERVAL_DAY_TIME 8
+#define ARROW_CONVERT_INTERVAL_YEAR_MONTH 9
+#define ARROW_CONVERT_LONGVARBINARY 10
+#define ARROW_CONVERT_LONGVARCHAR 11
+#define ARROW_CONVERT_NUMERIC 12
+#define ARROW_CONVERT_REAL 13
+#define ARROW_CONVERT_SMALLINT 14
+#define ARROW_CONVERT_TIME 15
+#define ARROW_CONVERT_TIMESTAMP 16
+#define ARROW_CONVERT_TINYINT 17
+#define ARROW_CONVERT_VARBINARY 18
+#define ARROW_CONVERT_VARCHAR 19
+
+namespace {
+// Return the corresponding field in SQLGetInfo's SQL_CONVERT_* field
+// types for the given Arrow SqlConvert enum value.
+//
+// The caller is responsible for casting the result to a uint16. Note
+// that -1 is returned if there's no corresponding entry.
+int32_t GetInfoTypeForArrowConvertEntry(int32_t convert_entry) {
+  switch (convert_entry) {
+case ARROW_CONVERT_BIGINT:
+  return SQL_CONVERT_BIGINT;
+case ARROW_CONVERT_BINARY:
+  return SQL_CONVERT_BINARY;
+case ARROW_CONVERT_BIT:
+  return SQL_CONVERT_BIT;
+case ARROW_CONVERT_CHAR:
+  return SQL_CONVERT_CHAR;
+case ARROW_CONVERT_DATE:
+  return SQL_CONVERT_DATE;
+case ARROW_CONVERT_DECIMAL:
+  return SQL_CONVERT_DECIMAL;
+case ARROW_CONVERT_FLOAT:
+  return SQL_CONVERT_FLOAT;
+case ARROW_CONVERT_INTEGER:
+  return SQL_CONVERT_INTEGER;
+case ARROW_CONVERT_INTERVAL_DAY_TIME:
+  return SQL_CONVERT_INTERVAL_DAY_TIME;
+case ARROW_CONVERT_INTERVAL_YEAR_MONTH:
+  return SQL_CONVERT_INTERVAL_YEAR_MONTH;
+case ARROW_CONVERT_LONGVARBINARY:
+  return SQL_CONVERT_LONGVARBINARY;
+case ARROW_CONVERT_LONGVARCHAR:
+  return SQL_CONVERT_LONGVARCHAR;
+case ARROW_CONVERT_NUMERIC:
+  return SQL_CONVERT_NUMERIC;
+case ARROW_CONVERT_REAL:
+  return SQL_CONVERT_REAL;
+case ARROW_CONVERT_SMALLINT:
+  return SQL_CONVERT_SMALLINT;
+case ARROW_CONVERT_TIME:
+  return SQL_CONVERT_TIME;
+case ARROW_CONVERT_TIMESTAMP:
+  return SQL_CONVERT_TIMESTAMP;
+case ARROW_CONVERT_TINYINT:
+  return SQL_CONVERT_TINYINT;
+case ARROW_CONVERT_VARBINARY:
+  return SQL_CONVERT_VARBINARY;
+case ARROW_CONVERT_VARCHAR:
+  return SQL_CONVERT_VARCHAR;
+  }
+  // Arbitrarily return a negative v

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-25 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2380440051


##
cpp/src/arrow/flight/sql/odbc/flight_sql/json_converter.cc:
##
@@ -0,0 +1,326 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/json_converter.h"
+
+#include 
+#include 
+#include 
+#include "arrow/builder.h"
+#include "arrow/flight/sql/odbc/flight_sql/utils.h"
+#include "arrow/scalar.h"
+#include "arrow/visitor.h"
+
+using arrow::Status;
+
+using boost::beast::detail::base64::encode;
+using boost::beast::detail::base64::encoded_size;
+namespace base64 = boost::beast::detail::base64;
+
+using driver::flight_sql::ThrowIfNotOK;
+
+namespace {
+template 
+Status ConvertScalarToStringAndWrite(const ScalarT& scalar,
+ 
rapidjson::Writer& writer) {
+  ARROW_ASSIGN_OR_RAISE(auto string_scalar, scalar.CastTo(arrow::utf8()))
+  const auto& view = 
reinterpret_cast(string_scalar.get())->view();
+  writer.String(view.data(), view.length(), true);
+  return Status::OK();
+}
+
+template 
+Status ConvertBinaryToBase64StringAndWrite(
+const BinaryScalarT& scalar, rapidjson::Writer& 
writer) {
+  const auto& view = scalar.view();
+  size_t encoded_size = base64::encoded_size(view.length());
+  std::vector encoded(std::max(encoded_size, static_cast(1)));
+  base64::encode(&encoded[0], view.data(), view.length());
+  writer.String(&encoded[0], encoded_size, true);
+  return Status::OK();
+}
+
+template 
+Status WriteListScalar(const ListScalarT& scalar,
+   rapidjson::Writer& writer,
+   arrow::ScalarVisitor* visitor) {
+  writer.StartArray();
+  for (int64_t i = 0; i < scalar.value->length(); ++i) {
+if (scalar.value->IsNull(i)) {
+  writer.Null();
+} else {
+  const auto& result = scalar.value->GetScalar(i);
+  ThrowIfNotOK(result.status());
+  ThrowIfNotOK(result.ValueOrDie()->Accept(visitor));
+}
+  }
+
+  writer.EndArray();
+  return Status::OK();
+}
+
+class ScalarToJson : public arrow::ScalarVisitor {
+ private:
+  rapidjson::StringBuffer string_buffer_;
+  rapidjson::Writer writer_{string_buffer_};
+
+ public:
+  void Reset() {
+string_buffer_.Clear();
+writer_.Reset(string_buffer_);
+  }
+
+  std::string ToString() { return string_buffer_.GetString(); }
+
+  Status Visit(const arrow::NullScalar& scalar) override {
+writer_.Null();
+
+return Status::OK();
+  }
+
+  Status Visit(const arrow::BooleanScalar& scalar) override {
+writer_.Bool(scalar.value);
+
+return Status::OK();
+  }
+
+  Status Visit(const arrow::Int8Scalar& scalar) override {
+writer_.Int(scalar.value);
+
+return Status::OK();
+  }
+
+  Status Visit(const arrow::Int16Scalar& scalar) override {
+writer_.Int(scalar.value);
+
+return Status::OK();
+  }
+
+  Status Visit(const arrow::Int32Scalar& scalar) override {
+writer_.Int(scalar.value);
+
+return Status::OK();
+  }
+
+  Status Visit(const arrow::Int64Scalar& scalar) override {
+writer_.Int64(scalar.value);

Review Comment:
   I see from tests, rapidJSON can still convert the max and min int64 to 
strings, so rapidJSON seems to support all numbers within the range of 
(9223372036854775807,-9223372036854775808). For example the following test 
works:
   
https://github.com/apache/arrow/blob/00439d104804fca889016d054c03ff6ba9d5560f/cpp/src/arrow/flight/sql/odbc/flight_sql/json_converter_test.cc#L76-L82



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-25 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2380390763


##
cpp/src/arrow/flight/sql/odbc/flight_sql/utils.h:
##
@@ -0,0 +1,124 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 

+#include 

+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+
+typedef std::function(const 
std::shared_ptr&)>
+ArrayConvertTask;
+
+using std::optional;
+
+inline void ThrowIfNotOK(const arrow::Status& status) {
+  if (!status.ok()) {
+throw odbcabstraction::DriverException(status.message());
+  }
+}
+
+template 
+inline bool CheckIfSetToOnlyValidValue(const AttributeTypeT& value, T 
allowed_value) {
+  return boost::get(value) == allowed_value;
+}
+
+template 
+arrow::Status AppendToBuilder(BUILDER& builder, optional opt_value) {
+  if (opt_value) {
+return builder.Append(*opt_value);
+  } else {
+return builder.AppendNull();
+  }
+}
+
+template 
+arrow::Status AppendToBuilder(BUILDER& builder, T value) {
+  return builder.Append(value);
+}
+
+odbcabstraction::SqlDataType GetDataTypeFromArrowField_V3(
+const std::shared_ptr& field, bool useWideChar);
+
+odbcabstraction::SqlDataType EnsureRightSqlCharType(
+odbcabstraction::SqlDataType data_type, bool useWideChar);
+
+int16_t ConvertSqlDataTypeFromV3ToV2(int16_t data_type_v3);

Review Comment:
   The function is not using an enum because imo enums aren't necessary in this 
case. The ODBC data type returned here serves as a return output value (to the 
BI tools etc) only, not for working with other parts of the driver. 
   The ODBC data type values are fixed macros defined inside the system ODBC 
library (e.g., `sqlext.h`):
   
   
https://github.com/microsoft/ODBCTest/blob/0d629c7e4ff7b01398a5ac71d20c43362d0f43bf/ODBC%204.0/inc/sqlext.h#L463-L464



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-25 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2380378199


##
cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_statement.cc:
##
@@ -0,0 +1,787 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include 

+
+#include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using ODBC::DescriptorRecord;
+using ODBC::ODBCConnection;
+using ODBC::ODBCDescriptor;
+using ODBC::ODBCStatement;
+
+using driver::odbcabstraction::DriverException;
+using driver::odbcabstraction::ResultSetMetadata;
+using driver::odbcabstraction::Statement;
+
+namespace {
+void DescriptorToHandle(SQLPOINTER output, ODBCDescriptor* descriptor,
+SQLINTEGER* lenPtr) {
+  if (output) {
+SQLHANDLE* outputHandle = static_cast(output);
+*outputHandle = reinterpret_cast(descriptor);
+  }
+  if (lenPtr) {
+*lenPtr = sizeof(SQLHANDLE);
+  }
+}
+
+size_t GetLength(const DescriptorRecord& record) {
+  switch (record.m_type) {
+case SQL_C_CHAR:
+case SQL_C_WCHAR:
+case SQL_C_BINARY:
+  return record.m_length;
+
+case SQL_C_BIT:
+case SQL_C_TINYINT:
+case SQL_C_STINYINT:
+case SQL_C_UTINYINT:
+  return sizeof(SQLSCHAR);
+
+case SQL_C_SHORT:
+case SQL_C_SSHORT:
+case SQL_C_USHORT:
+  return sizeof(SQLSMALLINT);
+
+case SQL_C_LONG:
+case SQL_C_SLONG:
+case SQL_C_ULONG:
+case SQL_C_FLOAT:
+  return sizeof(SQLINTEGER);
+
+case SQL_C_SBIGINT:
+case SQL_C_UBIGINT:
+case SQL_C_DOUBLE:
+  return sizeof(SQLBIGINT);
+
+case SQL_C_NUMERIC:
+  return sizeof(SQL_NUMERIC_STRUCT);
+
+case SQL_C_DATE:
+case SQL_C_TYPE_DATE:
+  return sizeof(SQL_DATE_STRUCT);
+
+case SQL_C_TIME:
+case SQL_C_TYPE_TIME:
+  return sizeof(SQL_TIME_STRUCT);
+
+case SQL_C_TIMESTAMP:
+case SQL_C_TYPE_TIMESTAMP:
+  return sizeof(SQL_TIMESTAMP_STRUCT);
+
+case SQL_C_INTERVAL_DAY:
+case SQL_C_INTERVAL_DAY_TO_HOUR:
+case SQL_C_INTERVAL_DAY_TO_MINUTE:
+case SQL_C_INTERVAL_DAY_TO_SECOND:
+case SQL_C_INTERVAL_HOUR:
+case SQL_C_INTERVAL_HOUR_TO_MINUTE:
+case SQL_C_INTERVAL_HOUR_TO_SECOND:
+case SQL_C_INTERVAL_MINUTE:
+case SQL_C_INTERVAL_MINUTE_TO_SECOND:
+case SQL_C_INTERVAL_SECOND:
+case SQL_C_INTERVAL_YEAR:
+case SQL_C_INTERVAL_YEAR_TO_MONTH:
+case SQL_C_INTERVAL_MONTH:
+  return sizeof(SQL_INTERVAL_STRUCT);
+default:
+  return record.m_length;
+  }
+}
+
+SQLSMALLINT getCTypeForSQLType(const DescriptorRecord& record) {
+  switch (record.m_conciseType) {
+case SQL_CHAR:
+case SQL_VARCHAR:
+case SQL_LONGVARCHAR:
+  return SQL_C_CHAR;
+
+case SQL_WCHAR:
+case SQL_WVARCHAR:
+case SQL_WLONGVARCHAR:
+  return SQL_C_WCHAR;
+
+case SQL_BINARY:
+case SQL_VARBINARY:
+case SQL_LONGVARBINARY:
+  return SQL_C_BINARY;
+
+case SQL_TINYINT:
+  return record.m_unsigned ? SQL_C_UTINYINT : SQL_C_STINYINT;
+
+case SQL_SMALLINT:
+  return record.m_unsigned ? SQL_C_USHORT : SQL_C_SSHORT;
+
+case SQL_INTEGER:
+  return record.m_unsigned ? SQL_C_ULONG : SQL_C_SLONG;
+
+case SQL_BIGINT:
+  return record.m_unsigned ? SQL_C_UBIGINT : SQL_C_SBIGINT;
+
+case SQL_REAL:
+  return SQL_C_FLOAT;
+
+case SQL_FLOAT:
+case SQL_DOUBLE:
+  return SQL_C_DOUBLE;
+
+case SQL_DATE:
+case SQL_TYPE_DATE:
+  return SQL_C_TYPE_DATE;
+
+case SQL_TIME:
+case SQL_TYPE_TIME:
+  return SQL_C_TYPE_TIME;
+
+case SQL_TIMESTAMP:
+case SQL_TYPE_TIMESTAMP:
+  return SQL_C_TYPE_TIMESTAMP;
+
+case SQL_C_INTERVAL_DAY:
+  return SQL_INTERVAL_DAY;
+case SQL_C_INTERVAL_DAY_TO_HOUR:
+  return SQL_INTERVAL_DAY_TO_HOUR;
+case SQL_C_INTERVAL_DAY_TO_MINUTE:
+  return SQL_INTERVAL_DAY_TO_MINUTE;
+case SQL_C_INTERVAL_DAY_TO_SECOND:
+  return SQL_INTERVAL_DAY_TO_SECOND;
+case SQL_C_INTERVAL_HOUR:
+  return SQL_INTERVAL_HOUR;
+case SQL_C_INTERVAL_HOUR_TO_MINUTE:
+  return SQL_INTERVAL_HOUR_TO_MINUTE;
+case SQL_C_INT

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-25 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2380373722


##
cpp/src/arrow/flight/sql/odbc/flight_sql/config/configuration.cc:
##
@@ -0,0 +1,181 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include 
"arrow/flight/sql/odbc/flight_sql/include/flight_sql/config/configuration.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+namespace config {
+
+static const char DEFAULT_DSN[] = "Apache Arrow Flight SQL";
+static const char DEFAULT_ENABLE_ENCRYPTION[] = TRUE_STR;
+static const char DEFAULT_USE_CERT_STORE[] = TRUE_STR;
+static const char DEFAULT_DISABLE_CERT_VERIFICATION[] = FALSE_STR;
+
+namespace {
+std::string ReadDsnString(const std::string& dsn, const std::string_view& key,
+  const std::string& dflt = "") {
+#define BUFFER_SIZE (1024)
+  std::vector buf(BUFFER_SIZE);
+
+  std::string key_str = std::string(key);
+  int ret =
+  SQLGetPrivateProfileString(dsn.c_str(), key_str.c_str(), dflt.c_str(), 
buf.data(),
+ static_cast(buf.size()), "ODBC.INI");
+
+  if (ret > BUFFER_SIZE) {
+// If there wasn't enough space, try again with the right size buffer.
+buf.resize(ret + 1);
+ret =
+SQLGetPrivateProfileString(dsn.c_str(), key_str.c_str(), dflt.c_str(), 
buf.data(),
+   static_cast(buf.size()), "ODBC.INI");
+  }
+
+  return std::string(buf.data(), ret);
+}
+
+void RemoveAllKnownKeys(std::vector& keys) {
+  // Remove all known DSN keys from the passed in set of keys, case 
insensitively.
+  keys.erase(std::remove_if(keys.begin(), keys.end(),
+[&](auto& x) {
+  return std::find_if(
+ FlightSqlConnection::ALL_KEYS.begin(),
+ FlightSqlConnection::ALL_KEYS.end(),
+ [&](auto& s) { return 
boost::iequals(x, s); }) !=
+ FlightSqlConnection::ALL_KEYS.end();
+}),
+ keys.end());
+}
+
+std::vector ReadAllKeys(const std::string& dsn) {
+  std::vector buf(BUFFER_SIZE);
+
+  int ret = SQLGetPrivateProfileString(dsn.c_str(), NULL, "", buf.data(),
+   static_cast(buf.size()), 
"ODBC.INI");
+
+  if (ret > BUFFER_SIZE) {
+// If there wasn't enough space, try again with the right size buffer.
+buf.resize(ret + 1);
+ret = SQLGetPrivateProfileString(dsn.c_str(), NULL, "", buf.data(),
+ static_cast(buf.size()), "ODBC.INI");
+  }
+
+  // When you pass NULL to SQLGetPrivateProfileString it gives back a \0 
delimited list of
+  // all the keys. The below loop simply tokenizes all the keys and places 
them into a
+  // vector.
+  std::vector keys;
+  char* begin = buf.data();
+  while (begin && *begin != '\0') {
+char* cur;
+for (cur = begin; *cur != '\0'; ++cur) {
+}
+keys.emplace_back(begin, cur);
+begin = ++cur;
+  }
+  return keys;
+}
+}  // namespace
+
+Configuration::Configuration() {
+  // No-op.
+}
+
+Configuration::~Configuration() {
+  // No-op.
+}
+
+void Configuration::LoadDefaults() {
+  Set(FlightSqlConnection::DSN, DEFAULT_DSN);
+  Set(FlightSqlConnection::USE_ENCRYPTION, DEFAULT_ENABLE_ENCRYPTION);
+  Set(FlightSqlConnection::USE_SYSTEM_TRUST_STORE, DEFAULT_USE_CERT_STORE);
+  Set(FlightSqlConnection::DISABLE_CERTIFICATE_VERIFICATION,
+  DEFAULT_DISABLE_CERT_VERIFICATION);
+}
+
+void Configuration::LoadDsn(const std::string& dsn) {
+  Set(FlightSqlConnection::DSN, dsn);
+  Set(FlightSqlConnection::HOST, ReadDsnString(dsn, 
FlightSqlConnection::HOST));
+  Set(FlightSqlConnection::PORT, ReadDsnString(dsn, 
FlightSqlConnection::PORT));
+  Set(FlightSqlConnection::TOKEN, ReadDsnString(dsn, 
FlightSqlConnection::TOKEN));
+  Set(FlightSqlConnection::UID, ReadDsnString(dsn, FlightSqlConnection::UID));
+  Set(FlightSqlConnection::PWD, ReadDsnString(dsn, FlightSqlConnection::PWD));
+  Set(FlightSqlConnecti

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-25 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2380245113


##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/types.h:
##
@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+#include "arrow/array.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/exceptions.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+
+namespace driver {
+namespace flight_sql {
+
+using arrow::Array;
+using odbcabstraction::CDataType;
+
+class FlightSqlResultSet;
+
+struct ColumnBinding {
+  void* buffer;
+  ssize_t* strlen_buffer;
+  size_t buffer_length;
+  CDataType target_type;
+  int precision;
+  int scale;
+
+  ColumnBinding() = default;
+
+  ColumnBinding(CDataType target_type, int precision, int scale, void* buffer,
+size_t buffer_length, ssize_t* strlen_buffer)
+  : target_type(target_type),
+precision(precision),
+scale(scale),
+buffer(buffer),
+buffer_length(buffer_length),
+strlen_buffer(strlen_buffer) {}
+};
+
+/// \brief Accessor interface meant to provide a way of populating data of a
+/// single column to buffers bound by `ColumnarResultSet::BindColumn`.
+class Accessor {
+ public:
+  const CDataType target_type_;
+
+  explicit Accessor(CDataType target_type) : target_type_(target_type) {}
+
+  virtual ~Accessor() = default;
+
+  /// \brief Populates next cells
+  virtual size_t GetColumnarData(ColumnBinding* binding, int64_t starting_row,
+ size_t cells, int64_t& value_offset,
+ bool update_value_offset,
+ odbcabstraction::Diagnostics& diagnostics,
+ uint16_t* row_status_array) = 0;
+
+  virtual size_t GetCellLength(ColumnBinding* binding) const = 0;
+};
+
+template 
+class FlightSqlAccessor : public Accessor {
+ public:
+  explicit FlightSqlAccessor(Array* array)
+  : Accessor(TARGET_TYPE),
+array_(arrow::internal::checked_cast(array)) {}
+
+  size_t GetColumnarData(ColumnBinding* binding, int64_t starting_row, size_t 
cells,
+ int64_t& value_offset, bool update_value_offset,
+ odbcabstraction::Diagnostics& diagnostics,
+ uint16_t* row_status_array) override {
+return static_cast(this)->GetColumnarData_impl(
+binding, starting_row, cells, value_offset, update_value_offset, 
diagnostics,
+row_status_array);
+  }
+
+  size_t GetCellLength(ColumnBinding* binding) const override {
+return static_cast(this)->GetCellLength_impl(binding);
+  }
+
+ protected:
+  size_t GetColumnarData_impl(ColumnBinding* binding, int64_t starting_row, 
int64_t cells,
+  int64_t& value_offset, bool update_value_offset,
+  odbcabstraction::Diagnostics& diagnostics,
+  uint16_t* row_status_array) {
+for (int64_t i = 0; i < cells; ++i) {
+  int64_t current_arrow_row = starting_row + i;
+  if (array_->IsNull(current_arrow_row)) {
+if (binding->strlen_buffer) {
+  binding->strlen_buffer[i] = odbcabstraction::NULL_DATA;
+} else {
+  throw odbcabstraction::NullWithoutIndicatorException();
+}
+  } else {
+// TODO: Optimize this by creating different versions of MoveSingleCell
+// depending on if strlen_buffer is null.
+auto row_status = MoveSingleCell(binding, current_arrow_row, i, 
value_offset,
+ update_value_offset, diagnostics);
+if (row_status_array) {
+  row_status_array[i] = row_status;
+}
+  }
+}
+
+return static_cast(cells);
+  }
+
+  inline ARROW_ARRAY* GetArray() { return array_; }
+
+ private:
+  ARROW_ARRAY* array_;
+
+  odbcabstraction::RowStatus MoveSingleCell(ColumnBinding* bin

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-24 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2373577249


##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/common.h:
##
@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include "arrow/array.h"
+#include "arrow/flight/sql/odbc/flight_sql/accessors/types.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+#include "arrow/scalar.h"
+
+namespace driver {
+namespace flight_sql {
+
+template 
+inline size_t CopyFromArrayValuesToBinding(ARRAY_TYPE* array, ColumnBinding* 
binding,
+   int64_t starting_row, int64_t 
cells) {
+  constexpr ssize_t element_size = sizeof(typename ARRAY_TYPE::value_type);
+
+  if (binding->strlen_buffer) {
+for (int64_t i = 0; i < cells; ++i) {
+  int64_t current_row = starting_row + i;
+  if (array->IsNull(current_row)) {
+binding->strlen_buffer[i] = odbcabstraction::NULL_DATA;
+  } else {
+binding->strlen_buffer[i] = element_size;
+  }
+}
+  } else {
+// Duplicate this loop to avoid null checks within the loop.
+for (int64_t i = starting_row; i < starting_row + cells; ++i) {
+  if (array->IsNull(i)) {
+throw odbcabstraction::NullWithoutIndicatorException();
+  }
+}

Review Comment:
   Yea this comment is a bit confusing. I am rewording it to `Duplicate above 
for loop to exit early when null value is found`. 
   - I am putting together a PR to address this comment (and other minor 
comments)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-23 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2373605307


##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/types.h:
##
@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+#include "arrow/array.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/exceptions.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+
+namespace driver {
+namespace flight_sql {
+
+using arrow::Array;
+using odbcabstraction::CDataType;
+
+class FlightSqlResultSet;
+
+struct ColumnBinding {
+  void* buffer;
+  ssize_t* strlen_buffer;
+  size_t buffer_length;
+  CDataType target_type;
+  int precision;
+  int scale;
+
+  ColumnBinding() = default;
+
+  ColumnBinding(CDataType target_type, int precision, int scale, void* buffer,
+size_t buffer_length, ssize_t* strlen_buffer)
+  : target_type(target_type),
+precision(precision),
+scale(scale),
+buffer(buffer),
+buffer_length(buffer_length),
+strlen_buffer(strlen_buffer) {}
+};
+
+/// \brief Accessor interface meant to provide a way of populating data of a
+/// single column to buffers bound by `ColumnarResultSet::BindColumn`.
+class Accessor {
+ public:
+  const CDataType target_type_;
+
+  explicit Accessor(CDataType target_type) : target_type_(target_type) {}
+
+  virtual ~Accessor() = default;
+
+  /// \brief Populates next cells
+  virtual size_t GetColumnarData(ColumnBinding* binding, int64_t starting_row,
+ size_t cells, int64_t& value_offset,
+ bool update_value_offset,
+ odbcabstraction::Diagnostics& diagnostics,
+ uint16_t* row_status_array) = 0;

Review Comment:
   Yes, `value_offset` is an in-out parameter. If `value_offset` is set to -1 
after `GetColumnarData` is called, it indicates to the driver that the cell 
value is null. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-09-23 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2373577249


##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/common.h:
##
@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include "arrow/array.h"
+#include "arrow/flight/sql/odbc/flight_sql/accessors/types.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+#include "arrow/scalar.h"
+
+namespace driver {
+namespace flight_sql {
+
+template 
+inline size_t CopyFromArrayValuesToBinding(ARRAY_TYPE* array, ColumnBinding* 
binding,
+   int64_t starting_row, int64_t 
cells) {
+  constexpr ssize_t element_size = sizeof(typename ARRAY_TYPE::value_type);
+
+  if (binding->strlen_buffer) {
+for (int64_t i = 0; i < cells; ++i) {
+  int64_t current_row = starting_row + i;
+  if (array->IsNull(current_row)) {
+binding->strlen_buffer[i] = odbcabstraction::NULL_DATA;
+  } else {
+binding->strlen_buffer[i] = element_size;
+  }
+}
+  } else {
+// Duplicate this loop to avoid null checks within the loop.
+for (int64_t i = starting_row; i < starting_row + cells; ++i) {
+  if (array->IsNull(i)) {
+throw odbcabstraction::NullWithoutIndicatorException();
+  }
+}

Review Comment:
   Yea this comment is a bit confusing. I am rewording it to `Duplicate above 
for loop to exit early when null value is found`. 
   - I am putting together a PR to address this comment



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-08-28 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2308498985


##
cpp/src/arrow/flight/sql/odbc/flight_sql/utils.h:
##
@@ -0,0 +1,124 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 

+#include 

+#include 
+#include 
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+
+typedef std::function(const 
std::shared_ptr&)>
+ArrayConvertTask;
+
+using std::optional;
+
+inline void ThrowIfNotOK(const arrow::Status& status) {
+  if (!status.ok()) {
+throw odbcabstraction::DriverException(status.message());
+  }
+}
+
+template 
+inline bool CheckIfSetToOnlyValidValue(const AttributeTypeT& value, T 
allowed_value) {
+  return boost::get(value) == allowed_value;
+}
+
+template 
+arrow::Status AppendToBuilder(BUILDER& builder, optional opt_value) {
+  if (opt_value) {
+return builder.Append(*opt_value);
+  } else {
+return builder.AppendNull();
+  }
+}
+
+template 
+arrow::Status AppendToBuilder(BUILDER& builder, T value) {
+  return builder.Append(value);
+}
+
+odbcabstraction::SqlDataType GetDataTypeFromArrowField_V3(

Review Comment:
   _V3 refers to ODBC Version 3 API backend function, whereas _V2 refers to 
ODBC version 2 API. Version 3 and Version 2 have slightly different behaviors. 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-08-28 Thread via GitHub


alinaliBQ commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2308475066


##
cpp/src/arrow/flight/sql/odbc/odbcabstraction/odbc_impl/odbc_statement.cc:
##
@@ -0,0 +1,787 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include 

+
+#include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 

+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+using ODBC::DescriptorRecord;
+using ODBC::ODBCConnection;
+using ODBC::ODBCDescriptor;
+using ODBC::ODBCStatement;
+
+using driver::odbcabstraction::DriverException;
+using driver::odbcabstraction::ResultSetMetadata;
+using driver::odbcabstraction::Statement;
+
+namespace {
+void DescriptorToHandle(SQLPOINTER output, ODBCDescriptor* descriptor,
+SQLINTEGER* lenPtr) {
+  if (output) {
+SQLHANDLE* outputHandle = static_cast(output);
+*outputHandle = reinterpret_cast(descriptor);
+  }
+  if (lenPtr) {
+*lenPtr = sizeof(SQLHANDLE);
+  }
+}
+
+size_t GetLength(const DescriptorRecord& record) {
+  switch (record.m_type) {
+case SQL_C_CHAR:
+case SQL_C_WCHAR:
+case SQL_C_BINARY:
+  return record.m_length;
+
+case SQL_C_BIT:
+case SQL_C_TINYINT:
+case SQL_C_STINYINT:
+case SQL_C_UTINYINT:
+  return sizeof(SQLSCHAR);
+
+case SQL_C_SHORT:
+case SQL_C_SSHORT:
+case SQL_C_USHORT:
+  return sizeof(SQLSMALLINT);
+
+case SQL_C_LONG:
+case SQL_C_SLONG:
+case SQL_C_ULONG:
+case SQL_C_FLOAT:
+  return sizeof(SQLINTEGER);
+
+case SQL_C_SBIGINT:
+case SQL_C_UBIGINT:
+case SQL_C_DOUBLE:
+  return sizeof(SQLBIGINT);
+
+case SQL_C_NUMERIC:
+  return sizeof(SQL_NUMERIC_STRUCT);
+
+case SQL_C_DATE:
+case SQL_C_TYPE_DATE:
+  return sizeof(SQL_DATE_STRUCT);
+
+case SQL_C_TIME:
+case SQL_C_TYPE_TIME:
+  return sizeof(SQL_TIME_STRUCT);
+
+case SQL_C_TIMESTAMP:
+case SQL_C_TYPE_TIMESTAMP:
+  return sizeof(SQL_TIMESTAMP_STRUCT);
+
+case SQL_C_INTERVAL_DAY:
+case SQL_C_INTERVAL_DAY_TO_HOUR:
+case SQL_C_INTERVAL_DAY_TO_MINUTE:
+case SQL_C_INTERVAL_DAY_TO_SECOND:
+case SQL_C_INTERVAL_HOUR:
+case SQL_C_INTERVAL_HOUR_TO_MINUTE:
+case SQL_C_INTERVAL_HOUR_TO_SECOND:
+case SQL_C_INTERVAL_MINUTE:
+case SQL_C_INTERVAL_MINUTE_TO_SECOND:
+case SQL_C_INTERVAL_SECOND:
+case SQL_C_INTERVAL_YEAR:
+case SQL_C_INTERVAL_YEAR_TO_MONTH:
+case SQL_C_INTERVAL_MONTH:
+  return sizeof(SQL_INTERVAL_STRUCT);
+default:
+  return record.m_length;
+  }
+}
+
+SQLSMALLINT getCTypeForSQLType(const DescriptorRecord& record) {
+  switch (record.m_conciseType) {
+case SQL_CHAR:
+case SQL_VARCHAR:
+case SQL_LONGVARCHAR:
+  return SQL_C_CHAR;
+
+case SQL_WCHAR:
+case SQL_WVARCHAR:
+case SQL_WLONGVARCHAR:
+  return SQL_C_WCHAR;
+
+case SQL_BINARY:
+case SQL_VARBINARY:
+case SQL_LONGVARBINARY:
+  return SQL_C_BINARY;
+
+case SQL_TINYINT:
+  return record.m_unsigned ? SQL_C_UTINYINT : SQL_C_STINYINT;
+
+case SQL_SMALLINT:
+  return record.m_unsigned ? SQL_C_USHORT : SQL_C_SSHORT;
+
+case SQL_INTEGER:
+  return record.m_unsigned ? SQL_C_ULONG : SQL_C_SLONG;
+
+case SQL_BIGINT:
+  return record.m_unsigned ? SQL_C_UBIGINT : SQL_C_SBIGINT;
+
+case SQL_REAL:
+  return SQL_C_FLOAT;
+
+case SQL_FLOAT:
+case SQL_DOUBLE:
+  return SQL_C_DOUBLE;
+
+case SQL_DATE:
+case SQL_TYPE_DATE:
+  return SQL_C_TYPE_DATE;
+
+case SQL_TIME:
+case SQL_TYPE_TIME:
+  return SQL_C_TYPE_TIME;
+
+case SQL_TIMESTAMP:
+case SQL_TYPE_TIMESTAMP:
+  return SQL_C_TYPE_TIMESTAMP;
+
+case SQL_C_INTERVAL_DAY:
+  return SQL_INTERVAL_DAY;
+case SQL_C_INTERVAL_DAY_TO_HOUR:
+  return SQL_INTERVAL_DAY_TO_HOUR;
+case SQL_C_INTERVAL_DAY_TO_MINUTE:
+  return SQL_INTERVAL_DAY_TO_MINUTE;
+case SQL_C_INTERVAL_DAY_TO_SECOND:
+  return SQL_INTERVAL_DAY_TO_SECOND;
+case SQL_C_INTERVAL_HOUR:
+  return SQL_INTERVAL_HOUR;
+case SQL_C_INTERVAL_HOUR_TO_MINUTE:
+  return SQL_INTERVAL_HOUR_TO_MINUTE;
+case SQL_C_INT

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-28 Thread via GitHub


alinaliBQ commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2917193499

   @lidavidm Yes that's correct. What was merged is not a complete driver.
   Folks are welcomed to look at my team's in-progress ODBC driver at 
https://github.com/apache/arrow/pull/46099 for Windows (it is not tested for 
macOS/Linux); PR#46099 code is replacement for the GPL license code in the 
original Dremio ODBC driver. The PR#46099 will be continuously updated with 
more ODBC development commits. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-27 Thread via GitHub


lidavidm commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2915071410

   To be clear: what was donated and merged is _not_ a complete ODBC driver. 
The contributors are still working on the rest of the necessary pieces. 
(Dremio's original driver was a mix of GPL and Apache licensed code, and of 
course the GPL licensed code can't be used here.)


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-27 Thread via GitHub


hiroyuki-sato commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2915008390

   Hi, @jmao-denver 
   In my opinion, the ODBC driver has just been merged and there are issues 
like #46622 so I think it's better to wait a bit.
   Anyway, this PR already merged. So, If you have any problem. You would 
better to create a new discussion or issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-27 Thread via GitHub


jmao-denver commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2914901638

   Hi @hiroyuki-sato, yeah I deleted it because I realized that I just needed 
to turn CUDA off. The build on my MBP M2 still failed with what seemed to be 
compatibility issues caused by the version of the compiler I was using. I then 
went to try the build on Windows with the latest Visual Studio Community 
Edition (2022). It built but running the produced arrow_odbc_spi_imp_cli.exe 
generated a crash. I also briefly tried to build it on ubuntu and it failed 
there as well. 
   
   I was basically doing something like on MacOS and Ubuntu.
   ```
   cmake .. --preset ninja-debug-minimal -DARROW_FLIGHT_SQL_ODBC=ON
   cmake --build .
   ```
   and some additional options on Windows.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-27 Thread via GitHub


hiroyuki-sato commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2914863596

   Hello, @jmao-denver 
   
   I saw this part in my Gmail. but I can't find the comment in this PR.
   
   ```
   -- Looking for backtrace
   -- Looking for backtrace - found
   -- backtrace facility detected in default set of libraries
   -- Found Backtrace: /usr/include  
   CMake Error at /usr/share/cmake-3.28/Modules/FindCUDAToolkit.cmake:855 
(message):
 Could not find nvcc, please set CUDAToolkit_ROOT.
   Call Stack (most recent call first):
 src/arrow/gpu/CMakeLists.txt:44 (find_package)
   ```
   
   I'm using the following steps on my macOS.
   
   ```
   git clone https://github.com/apache/arrow/
   cd arrow/cpp
   mkdir build
   cd build
   cmake .. --preset ninja-debug-maximal \
 -DCMAKE_INSTALL_PREFIX=/tmp/local \
 -DARROW_CUDA=OFF \
 -DARROW_SKYHOOK=OFF \
 
-DCMAKE_OSX_SYSROOT=/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk
 \
 -DARROW_EXTRA_ERROR_CONTEXT=OFF
   cmake --build .
   ```
   
   You need to wait this PR merge. https://github.com/apache/arrow/pull/46622


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-27 Thread via GitHub


jmao-denver commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2912712356

   > > Congratulations on the merge!
   > > Got an error when trying the build it on Mac M2.
   > > ```shell
   > > ~/git/arrow/cpp/build main *1 ?1 ❯ cmake .. --preset ninja-debug-maximal
   > > ```
   > > 
   > > 
   > > 
   > >   
   > > 
   > > 
   > >   
   > > 
   > > 
   > > 
   > >   
   > > ```
   > > ...
   > > Vcpkg integrate step - DONE.
   > > -- Retrieving version from 
/Users/jianfengmao/git/arrow/cpp/build/_deps/azure_sdk-src/sdk/tables/azure-data-tables/src/private/package_version.hpp
   > > -- VERSION_MAJOR 1
   > > -- VERSION_MINOR 0
   > > -- VERSION_PATCH 0
   > > -- VERSION_PRERELEASE beta.4
   > > -- AZ_LIBRARY_VERSION
   > > CMake Error at 
/opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:227 
(message):
   > >   Could NOT find ODBC (missing: ODBC_INCLUDE_DIR)
   > > Call Stack (most recent call first):
   > >   
/opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:591 
(_FPHSA_FAILURE_MESSAGE)
   > >   /opt/homebrew/share/cmake/Modules/FindODBC.cmake:200 
(find_package_handle_standard_args)
   > >   cmake_modules/ThirdpartyToolchain.cmake:5584 (find_package)
   > >   CMakeLists.txt:618 (include)
   > > ```
   > 
   > Hi Jianfeng, installing the ODBC driver manager on your machine might help 
with the issue.
   
   Thanks Alina. Installing `unixodbc` got the build past the error but it ran 
into another issue both on MacOS M2 and Ubuntu.
   ```
   -- Retrieving version from 
/home/jianfengmao/git/arrow/cpp/build/_deps/azure_sdk-src/sdk/tables/azure-data-tables/src/private/package_version.hpp
   -- VERSION_MAJOR 1
   -- VERSION_MINOR 0
   -- VERSION_PATCH 0
   -- VERSION_PRERELEASE beta.4
   -- AZ_LIBRARY_VERSION 
   -- Found ODBC: /usr/lib/x86_64-linux-gnu/libodbc.so  
   -- All bundled static libraries: 
gflags::gflags_static;substrait;mimalloc::mimalloc;Crc32c::crc32c;google-cloud-cpp::storage;google-cloud-cpp::rest-internal;google-cloud-cpp::common;orc::orc;aws-cpp-sdk-identity-management;aws-cpp-sdk-sts;aws-cpp-sdk-cognito-identity;aws-cpp-sdk-s3;aws-cpp-sdk-core;AWS::aws-crt-cpp;AWS::aws-c-s3;AWS::aws-c-auth;AWS::aws-c-mqtt;AWS::aws-c-http;AWS::aws-c-compression;AWS::aws-c-sdkutils;AWS::aws-c-event-stream;AWS::aws-c-io;AWS::aws-c-cal;AWS::aws-checksums;AWS::aws-c-common;AWS::s2n-tls;Azure::azure-core;Azure::azure-identity;Azure::azure-storage-blobs;Azure::azure-storage-common;Azure::azure-storage-files-datalake
   -- CMAKE_C_FLAGS:   -Wall -Wno-conversion -Wno-sign-conversion -Wdate-time 
-Wimplicit-fallthrough -Wunused-result -fno-semantic-interposition -msse4.2 
   -- CMAKE_CXX_FLAGS:  -Wredundant-move -Wno-noexcept-type -Wno-self-move  
-fdiagnostics-color=always  -Wall -Wno-conversion -Wno-sign-conversion 
-Wdate-time -Wimplicit-fallthrough -Wunused-result -fno-semantic-interposition 
-msse4.2 
   -- CMAKE_C_FLAGS_DEBUG: -g -Werror -O0 -ggdb 
   -- CMAKE_CXX_FLAGS_DEBUG: -g -Werror -O0 -ggdb 
   -- Looking for backtrace
   -- Looking for backtrace - found
   -- backtrace facility detected in default set of libraries
   -- Found Backtrace: /usr/include  
   CMake Error at /usr/share/cmake-3.28/Modules/FindCUDAToolkit.cmake:855 
(message):
 Could not find nvcc, please set CUDAToolkit_ROOT.
   Call Stack (most recent call first):
 src/arrow/gpu/CMakeLists.txt:44 (find_package)
   
   
   -- Configuring incomplete, errors occurred!
   ➜  build git:(main) 
   
   ```
   I am following the steps on 
https://arrow.apache.org/docs/developers/cpp/building.html. Maybe I am missing 
something very obvious? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-23 Thread via GitHub


alinaliBQ commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2905397679

   > Congratulations on the merge!
   > 
   > Got an error when trying the build it on Mac M2.
   > 
   > ```shell
   > ~/git/arrow/cpp/build main *1 ?1 ❯ cmake .. --preset ninja-debug-maximal
   > ```
   > 
   > ```
   > ...
   > Vcpkg integrate step - DONE.
   > -- Retrieving version from 
/Users/jianfengmao/git/arrow/cpp/build/_deps/azure_sdk-src/sdk/tables/azure-data-tables/src/private/package_version.hpp
   > -- VERSION_MAJOR 1
   > -- VERSION_MINOR 0
   > -- VERSION_PATCH 0
   > -- VERSION_PRERELEASE beta.4
   > -- AZ_LIBRARY_VERSION
   > CMake Error at 
/opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:227 
(message):
   >   Could NOT find ODBC (missing: ODBC_INCLUDE_DIR)
   > Call Stack (most recent call first):
   >   
/opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:591 
(_FPHSA_FAILURE_MESSAGE)
   >   /opt/homebrew/share/cmake/Modules/FindODBC.cmake:200 
(find_package_handle_standard_args)
   >   cmake_modules/ThirdpartyToolchain.cmake:5584 (find_package)
   >   CMakeLists.txt:618 (include)
   > ```
   
   Hi Jianfeng, installing the ODBC driver manager on your machine might help 
with the issue. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-23 Thread via GitHub


alinaliBQ commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2905394054

   Hi @lidavidm, thanks for reviewing, we (Rob and I) will begin looking into 
those comments after the development in the ODBC layer (PR #46099) has 
finished. 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-22 Thread via GitHub


jmao-denver commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2901452970

   Congratulations on the merge!
   
   Got an error when trying the build it on Mac M2. 
   
   ```shell
   ~/git/arrow/cpp/build main *1 ?1 ❯ cmake .. --preset ninja-debug-maximal
   ```
   ```text
   ...
   Vcpkg integrate step - DONE.
   -- Retrieving version from 
/Users/jianfengmao/git/arrow/cpp/build/_deps/azure_sdk-src/sdk/tables/azure-data-tables/src/private/package_version.hpp
   -- VERSION_MAJOR 1
   -- VERSION_MINOR 0
   -- VERSION_PATCH 0
   -- VERSION_PRERELEASE beta.4
   -- AZ_LIBRARY_VERSION
   CMake Error at 
/opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:227 
(message):
 Could NOT find ODBC (missing: ODBC_INCLUDE_DIR)
   Call Stack (most recent call first):
 /opt/homebrew/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:591 
(_FPHSA_FAILURE_MESSAGE)
 /opt/homebrew/share/cmake/Modules/FindODBC.cmake:200 
(find_package_handle_standard_args)
 cmake_modules/ThirdpartyToolchain.cmake:5584 (find_package)
 CMakeLists.txt:618 (include)
   ```
   
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-21 Thread via GitHub


lidavidm commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2101365880


##
cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spi/statement.h:
##
@@ -0,0 +1,193 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace odbcabstraction {
+
+using boost::optional;
+
+class ResultSet;
+
+class ResultSetMetadata;
+
+/// \brief High-level representation of an ODBC statement.
+class Statement {
+ protected:
+  Statement() = default;
+
+ public:
+  virtual ~Statement() = default;
+
+  /// \brief Statement attributes that can be called at anytime.
+  TODO: Document attributes
+  enum StatementAttributeId {

Review Comment:
   enum class



##
cpp/src/arrow/flight/sql/odbc/flight_sql/get_info_cache.cc:
##
@@ -0,0 +1,1345 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/get_info_cache.h"
+
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+
+#include 
+#include 
+#include "arrow/array.h"
+#include "arrow/array/array_nested.h"
+#include "arrow/flight/sql/api.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/exceptions.h"
+#include "arrow/scalar.h"
+#include "arrow/type_fwd.h"
+
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_stream_chunk_buffer.h"
+#include "arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.h"
+#include "arrow/flight/sql/odbc/flight_sql/utils.h"
+
+// Aliases for entries in SqlInfoOptions::SqlInfo that are defined here
+// due to causing compilation errors conflicting with ODBC definitions.
+#define ARROW_SQL_IDENTIFIER_CASE 503
+#define ARROW_SQL_IDENTIFIER_QUOTE_CHAR 504
+#define ARROW_SQL_QUOTED_IDENTIFIER_CASE 505
+#define ARROW_SQL_KEYWORDS 508
+#define ARROW_SQL_NUMERIC_FUNCTIONS 509
+#define ARROW_SQL_STRING_FUNCTIONS 510
+#define ARROW_SQL_SYSTEM_FUNCTIONS 511
+#define ARROW_SQL_SCHEMA_TERM 529
+#define ARROW_SQL_PROCEDURE_TERM 530
+#define ARROW_SQL_CATALOG_TERM 531
+#define ARROW_SQL_MAX_COLUMNS_IN_GROUP_BY 544
+#define ARROW_SQL_MAX_COLUMNS_IN_INDEX 545
+#define ARROW_SQL_MAX_COLUMNS_IN_ORDER_BY 546
+#define ARROW_SQL_MAX_COLUMNS_IN_SELECT 547
+#define ARROW_SQL_MAX_COLUMNS_IN_TABLE 548
+#define ARROW_SQL_MAX_ROW_SIZE 555
+#define ARROW_SQL_MAX_TABLES_IN_SELECT 560
+
+#define ARROW_CONVERT_BIGINT 0
+#define ARROW_CONVERT_BINARY 1
+#define ARROW_CONVERT_BIT 2
+#define ARROW_CONVERT_CHAR 3
+#define ARROW_CONVERT_DATE 4
+#define ARROW_CONVERT_DECIMAL 5
+#define ARROW_CONVERT_FLOAT 6
+#define ARROW_CONVERT_INTEGER 7
+#define ARROW_CONVERT_INTERVAL_DAY_TIME 8
+#define ARROW_CONVERT_INTERVAL_YEAR_MONTH 9
+#define ARROW_CONVERT_LONGVARBINARY 10
+#define ARROW_CONVERT_LONGVARCHAR 11
+#define ARROW_CONVERT_NUMERIC 12
+#define ARROW_CONVERT_REAL 13
+#define ARROW_CONVERT_SMALLINT 14
+#define ARROW_CONVERT_TIME 15
+#define ARROW_CONVERT_TIMESTAMP 16
+#define ARROW_CONVERT_TINYINT 17
+#define ARROW_CONVERT_VARBINARY 18
+#define ARROW_CONVERT_VARCHAR 19
+
+namespace {
+// Return the corresponding field in SQLGetInfo's SQL_CONVERT_* field
+// types for the given Arrow SqlConvert enum value.
+//
+// The caller is responsible for casting the result to a uint16. Note
+// that -1 is returned if there's no corresponding entry.
+int32_t GetInfoTypeForArrowConvertEntry(int32_t convert_entry) {
+

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-21 Thread via GitHub


chrisfw commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2899187438

   Thanks for your reply @jduo.  I will definitely add an issue to request an 
enhancement adding the currently unsupported data types I have encounter.
   
   Regards,
   Chris


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-21 Thread via GitHub


jduo commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2898503225

   > Hi @jbonofre , @rscales, @alinaliBQ, This is **_really_** exciting news! I 
tried using the Dremio Flight SQL ODBC Driver against a ROAPI dataset, but I 
ran into an issue with the underlying use of the (relatively) new Utf8View data 
type by DataFusion (used by ROAPI). Can you tell me if that data type is 
supported by this driver?
   > 
   > Thank you! Regards, Chris
   
   Hi @chrisfw .
   
   This driver currently doesn't support Utf8View (it supports the subset of 
Arrow types that Dremio uses) but it'd be much easier to support now since it 
is updated with the Arrow codebase. (Likely just a matter of updating switch 
statements and adding test cases).  It'd be great if you added an issue for any 
additional types that you'd need.
   
   Note that this isn't a complete ODBC driver. It has most of the ODBC 
implementation logic and all the Flight SQL comunication code. -- @alinaliBQ 
and @rscales are completing the last bits required to implement the ODBC 
interface.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-21 Thread via GitHub


chrisfw commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2897708898

   Hi @jbonofre , @rscales, @alinaliBQ,
   This is **_really_** exciting news!  I tried using the Dremio Flight SQL 
ODBC Driver against a ROAPI dataset, but I ran into an issue with the 
underlying use of the (relatively) new Utf8View data type by DataFusion (used 
by ROAPI).  Can you tell me if that data type is supported by this driver?
   
   Thank you!
   Regards,
   Chris  


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-21 Thread via GitHub


conbench-apache-arrow[bot] commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2897284377

   After merging your PR, Conbench analyzed the 4 benchmarking runs that have 
been run so far on merge-commit ed36107ad869dd0db5c33a7c3e5484f66a461a4f.
   
   There were no benchmark performance regressions. 🎉
   
   The [full Conbench report](https://github.com/apache/arrow/runs/42619788781) 
has more details. It also includes information about 4 possible false positives 
for unstable benchmarks that are known to sometimes produce them.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2099321922


##
cpp/src/arrow/flight/sql/odbc/flight_sql/scalar_function_reporter.h:
##
@@ -0,0 +1,32 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+
+namespace driver {
+namespace flight_sql {
+
+void ReportSystemFunction(const std::string& function, uint32_t& 
current_sys_functions,
+  uint32_t& current_convert_functions);
+void ReportNumericFunction(const std::string& function, uint32_t& 
current_functions);

Review Comment:
   Can the purpose of these functions be documented? Even looking at the 
implementation it's not very clear



##
cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h:
##
@@ -0,0 +1,109 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+#include 

+#include 

+
+namespace driver {
+namespace odbcabstraction {
+class Diagnostics {
+ public:
+  struct DiagnosticsRecord {
+std::string msg_text_;
+std::string sql_state_;
+int32_t native_error_;
+  };
+
+ private:
+  std::vector error_records_;
+  std::vector warning_records_;
+  std::vector> owned_records_;
+  std::string vendor_;
+  std::string data_source_component_;
+  OdbcVersion version_;
+
+ public:
+  Diagnostics(std::string vendor, std::string data_source_component, 
OdbcVersion version);
+  void AddError(const DriverException& exception);
+  void AddWarning(std::string message, std::string sql_state, int32_t 
native_error);
+
+  /// \brief Add a pre-existing truncation warning.
+  inline void AddTruncationWarning() {
+static const std::unique_ptr TRUNCATION_WARNING(
+new DiagnosticsRecord{"String or binary data, right-truncated.", 
"01004",
+  ODBCErrorCodes_TRUNCATION_WARNING});
+warning_records_.push_back(TRUNCATION_WARNING.get());

Review Comment:
   I think this one doesn't need to be a unique_ptr? It shouldn't ever get 
moved.



##
cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement.cc:
##
@@ -0,0 +1,298 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_statement.h"
+#include 
+#include 
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_result_set.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_result_set_metadata.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_columns.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_tables.h"
+#include 
"arrow/flight/sql/odbc/flight_sql/flight_sql_statem

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2896642385

   I'll continue reviewing tomorrow.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2099266258


##
cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_ssl_config.h:
##
@@ -0,0 +1,62 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+
+/// \brief An Auxiliary class that holds all the information to perform
+///a SSL connection.
+class FlightSqlSslConfig {
+ public:
+  FlightSqlSslConfig(bool disableCertificateVerification, const std::string& 
trustedCerts,
+ bool systemTrustStore, bool useEncryption);
+
+  /// \brief  Tells if ssl is enabled. By default it will be true.
+  /// \return Whether ssl is enabled.
+  bool useEncryption() const;

Review Comment:
   Should be either UseEncryption or use_encryption (because this is a trivial 
getter), ditto for below



##
cpp/src/arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/logger.h:
##
@@ -0,0 +1,67 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+
+#include 

Review Comment:
   Once we move to C++20 (soon), we need to use stdlib format, not poke into 
spdlog's internals



##
cpp/src/arrow/flight/sql/odbc/flight_sql/json_converter.cc:
##
@@ -0,0 +1,326 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/json_converter.h"
+
+#include 
+#include 
+#include 
+#include "arrow/builder.h"
+#include "arrow/flight/sql/odbc/flight_sql/utils.h"
+#include "arrow/scalar.h"
+#include "arrow/visitor.h"
+
+using arrow::Status;
+
+using boost::beast::detail::base64::encode;
+using boost::beast::detail::base64::encoded_size;
+namespace base64 = boost::beast::detail::base64;
+
+using driver::flight_sql::ThrowIfNotOK;
+
+namespace {
+template 
+Status ConvertScalarToStringAndWrite(const ScalarT& scalar,
+ 
rapidjson::Writer& writer) {
+  ARROW_ASSIGN_OR_RAISE(auto string_scalar, scalar.CastTo(arrow::utf8()))
+  const auto& view = 
reinterpret_cast(string_scalar.get())->view();
+  writer.String(view.data(), view.length(), true);
+  return Status::OK();
+}
+
+template 
+Status ConvertBinaryToBase64StringAndWrite(
+const BinaryScalarT& scalar, rapidjson::Writer& 
writer) {
+  const auto& view = scalar.view();
+  size_t encoded_size = base64::encoded_size(view.length());
+  std::vector encoded(std::max(encoded_size, static_cast(1)));
+  base64::encode(&encoded[0], view.data(), view.length());
+  writer.String(&encoded[0], encoded_size, true);
+  return Status::OK();
+}
+
+template 
+Status W

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2099229799


##
cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_statement_get_tables.h:
##
@@ -0,0 +1,72 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include "arrow/flight/sql/client.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h"
+#include "arrow/flight/sql/odbc/flight_sql/record_batch_transformer.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/spi/result_set.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+#include "arrow/flight/types.h"
+#include "arrow/type.h"
+
+namespace driver {
+namespace flight_sql {
+
+using arrow::flight::FlightCallOptions;
+using arrow::flight::sql::FlightSqlClient;
+using odbcabstraction::MetadataSettings;
+using odbcabstraction::ResultSet;
+
+typedef struct {

Review Comment:
   Why the C-ism? Just `struct ColumnNames`.



##
cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_connection.cc:
##
@@ -0,0 +1,439 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h"
+
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/utils.h"
+
+#include "arrow/flight/client_cookie_middleware.h"
+#include "arrow/flight/sql/odbc/flight_sql/address_info.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_auth_method.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_ssl_config.h"
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_statement.h"
+#include "arrow/flight/sql/odbc/flight_sql/utils.h"
+#include "arrow/flight/types.h"
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/exceptions.h"
+
+#include 
+#include 
+
+#include "arrow/flight/sql/odbc/flight_sql/system_trust_store.h"
+
+#ifndef NI_MAXHOST
+#  define NI_MAXHOST 1025
+#endif
+
+namespace driver {
+namespace flight_sql {
+
+using arrow::Result;
+using arrow::Status;
+using arrow::flight::FlightCallOptions;
+using arrow::flight::FlightClient;
+using arrow::flight::FlightClientOptions;
+using arrow::flight::Location;
+using arrow::flight::TimeoutDuration;
+using arrow::flight::sql::FlightSqlClient;
+using driver::odbcabstraction::AsBool;
+using driver::odbcabstraction::CommunicationException;
+using driver::odbcabstraction::Connection;
+using driver::odbcabstraction::DriverException;
+using driver::odbcabstraction::OdbcVersion;
+using driver::odbcabstraction::Statement;
+
+const std::vector FlightSqlConnection::ALL_KEYS = {
+FlightSqlConnection::DSN,
+FlightSqlConnection::DRIVER,
+FlightSqlConnection::HOST,
+FlightSqlConnection::PORT,
+FlightSqlConnection::TOKEN,
+FlightSqlConnection::UID,
+FlightSqlConnection::USER_ID,
+FlightSqlConnection::PWD,
+FlightSqlConnection::USE_ENCRYPTION,
+FlightSqlConnection::TRUSTED_CERTS,
+FlightSqlConnection::USE_SYSTEM_TRUST_STORE,
+FlightSqlConnection::DISABLE_CERTIFICATE_VERIFICATION,
+FlightSqlConnection::STRING_COLUMN_LENGTH,
+FlightSqlConnection::USE_WIDE_CHAR,
+FlightSqlConnection::CHUNK_BUFFER_CAPACITY};
+
+namespace {
+
+#i

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2099211671


##
cpp/src/arrow/flight/sql/odbc/flight_sql/config/connection_string_parser.cc:
##
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include 
"arrow/flight/sql/odbc/flight_sql/include/flight_sql/config/connection_string_parser.h"
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+namespace config {
+
+ConnectionStringParser::ConnectionStringParser(Configuration& cfg) : cfg(cfg) {
+  // No-op.

Review Comment:
   Redundant comment



##
cpp/src/arrow/flight/sql/odbc/flight_sql/config/connection_string_parser.cc:
##
@@ -0,0 +1,102 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include 
"arrow/flight/sql/odbc/flight_sql/include/flight_sql/config/connection_string_parser.h"
+
+#include 
+#include 
+#include 
+#include 
+
+#include 
+#include 
+#include 
+#include 
+
+namespace driver {
+namespace flight_sql {
+namespace config {
+
+ConnectionStringParser::ConnectionStringParser(Configuration& cfg) : cfg(cfg) {
+  // No-op.
+}
+
+ConnectionStringParser::~ConnectionStringParser() {
+  // No-op.
+}
+
+void ConnectionStringParser::ParseConnectionString(const char* str, size_t len,
+   char delimiter) {
+  std::string connect_str(str, len);
+
+  while (connect_str.rbegin() != connect_str.rend() && *connect_str.rbegin() 
== 0)
+connect_str.erase(connect_str.size() - 1);

Review Comment:
   Instead of erasing, you can use a string_view to slice



##
cpp/src/arrow/flight/sql/odbc/flight_sql/flight_sql_connection_test.cc:
##
@@ -0,0 +1,211 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#include "arrow/flight/sql/odbc/flight_sql/flight_sql_connection.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+#include "arrow/flight/types.h"
+#include "gtest/gtest.h"
+
+namespace driver {
+namespace flight_sql {
+
+using arrow::flight::Location;
+using arrow::flight::TimeoutDuration;
+using odbcabstraction::Connection;
+
+TEST(AttributeTests, SetAndGetAttribute) {
+  FlightSqlConnection connection(odbcabstraction::V_3);
+  connection.SetClosed(false);
+
+  connection.SetAttribute(Connection::CONNECTION_TIMEOUT, 
static_cast(200));
+  const boost::optional firstValue =
+  connection.GetAttribute(Connection::CONNECTION_TIMEOUT);
+
+  EXPECT_TRUE(firstValue);
+
+  EXPECT_EQ(boost::get(*firstValue), static_cast(200));
+
+  connection.SetAttribute(Connection::CONNECTION_TIMEOUT, 
static_cast(300))

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm commented on code in PR #40939:
URL: https://github.com/apache/arrow/pull/40939#discussion_r2099116665


##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/main.h:
##


Review Comment:
   By codebase convention this should be called "api.h".



##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/common.h:
##
@@ -0,0 +1,64 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include "arrow/array.h"
+#include "arrow/flight/sql/odbc/flight_sql/accessors/types.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+#include "arrow/scalar.h"
+
+namespace driver {
+namespace flight_sql {

Review Comment:
   Let's fix the namespace: this should be something like 
`arrow::flight::sql::odbc`



##
cpp/src/arrow/flight/sql/odbc/flight_sql/accessors/types.h:
##
@@ -0,0 +1,148 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements.  See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership.  The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License.  You may obtain a copy of the License at
+//
+//   http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+#pragma once
+
+#include 
+#include 
+#include 
+
+#include "arrow/array.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/diagnostics.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/exceptions.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/platform.h"
+#include 
"arrow/flight/sql/odbc/odbcabstraction/include/odbcabstraction/types.h"
+
+namespace driver {
+namespace flight_sql {
+
+using arrow::Array;
+using odbcabstraction::CDataType;
+
+class FlightSqlResultSet;
+
+struct ColumnBinding {
+  void* buffer;
+  ssize_t* strlen_buffer;
+  size_t buffer_length;
+  CDataType target_type;
+  int precision;
+  int scale;
+
+  ColumnBinding() = default;
+
+  ColumnBinding(CDataType target_type, int precision, int scale, void* buffer,
+size_t buffer_length, ssize_t* strlen_buffer)
+  : target_type(target_type),
+precision(precision),
+scale(scale),
+buffer(buffer),
+buffer_length(buffer_length),
+strlen_buffer(strlen_buffer) {}
+};
+
+/// \brief Accessor interface meant to provide a way of populating data of a
+/// single column to buffers bound by `ColumnarResultSet::BindColumn`.
+class Accessor {
+ public:
+  const CDataType target_type_;
+
+  explicit Accessor(CDataType target_type) : target_type_(target_type) {}
+
+  virtual ~Accessor() = default;
+
+  /// \brief Populates next cells
+  virtual size_t GetColumnarData(ColumnBinding* binding, int64_t starting_row,
+ size_t cells, int64_t& value_offset,
+ bool update_value_offset,
+ odbcabstraction::Diagnostics& diagnostics,
+ uint16_t* row_status_array) = 0;
+
+  virtual size_t GetCellLength(ColumnBinding* binding) const = 0;
+};
+
+template 
+class FlightSqlAccessor : public Accessor {
+ public:
+  explicit FlightSqlAccessor(Array* array)
+  : Accessor(TARGET_TYPE),
+array_(arrow::internal::checked_cast(array)) {}
+
+  size_t GetColumnarData(ColumnBinding* binding, int64_t starting_row, size_t 
cells,
+ int64_t& value_offset, bool update_value_offset,
+ odbcabstraction::Diagnostics& diagnostics,
+ uint16_t* row_status_array) override {
+return static_cast(this)->Get

Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


lidavidm merged PR #40939:
URL: https://github.com/apache/arrow/pull/40939


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]



Re: [PR] GH-46522: [C++][FlightRPC] Add Arrow Flight SQL ODBC driver [arrow]

2025-05-20 Thread via GitHub


github-actions[bot] commented on PR #40939:
URL: https://github.com/apache/arrow/pull/40939#issuecomment-2896175562

   :warning: GitHub issue #46522 **has been automatically assigned in GitHub** 
to PR creator.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]