Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cpp/src/arrow/flight/sql/odbc/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -110,10 +110,10 @@ if(ARROW_FLIGHT_SQL_ODBC_INSTALLER)
set(CPACK_PACKAGE_NAME ${ODBC_PACKAGE_NAME})
set(CPACK_PACKAGE_VENDOR ${ODBC_PACKAGE_VENDOR})
set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "Apache Arrow Flight SQL ODBC Driver")
set(CPACK_PACKAGE_CONTACT "#TODO arrow maintainers")
set(CPACK_PACKAGE_CONTACT "#GH-47787 TODO arrow maintainers")

# TODO: set up `flight_sql_odbc_lib` component for macOS Installer
# TODO: set up `flight_sql_odbc_lib` component for Linux Installer
# GH-47876 TODO: set up `flight_sql_odbc_lib` component for macOS Installer
# GH-47877 TODO: set up `flight_sql_odbc_lib` component for Linux Installer
if(WIN32)
install(DIRECTORY "${BUILD_OUTPUT_ROOT_DIRECTORY}${CMAKE_BUILD_TYPE}/"
DESTINATION bin
Expand Down Expand Up @@ -156,8 +156,8 @@ if(ARROW_FLIGHT_SQL_ODBC_INSTALLER)
# Upgrade GUID is required to be unchanged for ODBC installer to upgrade
set(CPACK_WIX_UPGRADE_GUID "DBF27A18-F8BF-423F-9E3A-957414D52C4B")
endif()
# TODO: create macOS Installer using cpack
# TODO: create Linux Installer using cpack
# GH-47876 TODO: create macOS Installer using cpack
# GH-47877 TODO: create Linux Installer using cpack

# Load CPack after all CPACK* variables are set
include(CPack)
Expand Down
16 changes: 7 additions & 9 deletions cpp/src/arrow/flight/sql/odbc/odbc_api.cc
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,7 @@ SQLRETURN SQLGetDiagField(SQLSMALLINT handle_type, SQLHANDLE handle,
SQLSMALLINT rec_number, SQLSMALLINT diag_identifier,
SQLPOINTER diag_info_ptr, SQLSMALLINT buffer_length,
SQLSMALLINT* string_length_ptr) {
// TODO: Implement additional fields types
// https://github.com/apache/arrow/issues/46573
// GH-46573 TODO: Implement additional fields types
ARROW_LOG(DEBUG) << "SQLGetDiagFieldW called with handle_type: " << handle_type
<< ", handle: " << handle << ", rec_number: " << rec_number
<< ", diag_identifier: " << diag_identifier
Expand Down Expand Up @@ -345,7 +344,7 @@ SQLRETURN SQLGetDiagField(SQLSMALLINT handle_type, SQLHANDLE handle,
return SQL_SUCCESS;
}

// TODO implement return code function
// Driver manager implements SQL_DIAG_RETURNCODE
case SQL_DIAG_RETURNCODE: {
return SQL_SUCCESS;
}
Expand Down Expand Up @@ -789,11 +788,10 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,
<< static_cast<const void*>(out_connection_string_len)
<< ", driver_completion: " << driver_completion;

// TODO: Implement FILEDSN and SAVEFILE keywords according to the spec
// https://github.com/apache/arrow/issues/46449
// GH-46449 TODO: Implement FILEDSN and SAVEFILE keywords according to the spec

// TODO: Copy connection string properly in SQLDriverConnect according to the
// spec https://github.com/apache/arrow/issues/46560
// GH-46560 TODO: Copy connection string properly in SQLDriverConnect according to the
// spec

using ODBC::ODBCConnection;

Expand All @@ -810,8 +808,8 @@ SQLRETURN SQLDriverConnect(SQLHDBC conn, SQLHWND window_handle,

std::vector<std::string_view> missing_properties;

// TODO: Implement SQL_DRIVER_COMPLETE_REQUIRED in SQLDriverConnect according to the
// spec https://github.com/apache/arrow/issues/46448
// GH-46448 TODO: Implement SQL_DRIVER_COMPLETE_REQUIRED in SQLDriverConnect according
// to the spec
#if defined _WIN32 || defined _WIN64
// Load the DSN window according to driver_completion
if (driver_completion == SQL_DRIVER_PROMPT) {
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/accessors/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ class FlightSqlAccessor : public Accessor {
throw NullWithoutIndicatorException();
}
} else {
// TODO: Optimize this by creating different versions of MoveSingleCell
// GH-47849 TODO: Optimize this by creating different versions of MoveSingleCell
// depending on if str_len_buffer is null.
auto row_status = MoveSingleCell(binding, current_arrow_row, i, value_offset,
update_value_offset, diagnostics);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,8 @@ class NoOpAuthMethod : public FlightSqlAuthMethod {
FlightCallOptions& call_options) override {
// Do nothing

// TODO: implement NoOpAuthMethod to validate server address.
// GH-46733 TODO: implement NoOpAuthMethod to validate server address.
// Can use NoOpClientAuthHandler.
// https://github.com/apache/arrow/issues/46733
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ Location FlightSqlConnection::BuildLocation(
ThrowIfNotOK(Location::ForGrpcTls(host_name_info, port).Value(&location));
return location;
}
// TODO: We should log that we could not convert an IP to hostname here.
// GH-47852 TODO: We should log that we could not convert an IP to hostname here.
}
} catch (...) {
// This is expected. The Host attribute can be an IP or name, but make_address will
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,9 @@ std::shared_ptr<Schema> GetTablesReader::GetSchema() {
const arrow::Result<std::shared_ptr<Schema>>& result =
arrow::ipc::ReadSchema(&dataset_schema_reader, &in_memo);
if (!result.ok()) {
// TODO: Test and build the driver against a server that returns
// GH-46561 TODO: Test and build the driver against a server that returns
// complex types columns with the children
// types and handle the failure properly
// https://github.com/apache/arrow/issues/46561
return nullptr;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,19 +157,27 @@ size_t FlightSqlResultSetMetadata::GetLength(int column_position) {
}

std::string FlightSqlResultSetMetadata::GetLiteralPrefix(int column_position) {
// TODO: Flight SQL column metadata does not have this, should we add to the spec?
// GH-47853 TODO: use `ColumnMetadata` to get literal prefix after Flight SQL protocol
// adds support for it

// Flight SQL column metadata does not have literal prefix, empty string is returned
return "";
}

std::string FlightSqlResultSetMetadata::GetLiteralSuffix(int column_position) {
// TODO: Flight SQL column metadata does not have this, should we add to the spec?
// GH-47853 TODO: use `ColumnMetadata` to get literal suffix after Flight SQL protocol
// adds support for it

// Flight SQL column metadata does not have literal suffix, empty string is returned
return "";
}

std::string FlightSqlResultSetMetadata::GetLocalTypeName(int column_position) {
ColumnMetadata metadata = GetMetadata(schema_->field(column_position - 1));

// TODO: Is local type name the same as type name?
// Local type name is for display purpose only.
// Return type name as local type name as Flight SQL protocol doesn't have support for
// local type name.
return metadata.GetTypeName().ValueOrElse([] { return ""; });
}

Expand All @@ -192,7 +200,7 @@ size_t FlightSqlResultSetMetadata::GetOctetLength(int column_position) {

// Workaround to get the precision for Decimal and Numeric types, since server doesn't
// return it currently.
// TODO: Use the server precision when its fixed.
// GH-47854 TODO: Use the server precision when its fixed.
std::shared_ptr<DataType> arrow_type = field->type();
if (arrow_type->id() == Type::DECIMAL128) {
int32_t precision = util::GetDecimalTypePrecision(arrow_type);
Expand Down Expand Up @@ -220,7 +228,6 @@ Updatability FlightSqlResultSetMetadata::GetUpdatable(int column_position) {
bool FlightSqlResultSetMetadata::IsAutoUnique(int column_position) {
ColumnMetadata metadata = GetMetadata(schema_->field(column_position - 1));

// TODO: Is AutoUnique equivalent to AutoIncrement?
return metadata.GetIsAutoIncrement().ValueOrElse([] { return false; });
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -101,10 +101,9 @@ Result<std::shared_ptr<RecordBatch>> TransformInner(
const auto& table_name = reader.GetTableName();
const std::shared_ptr<Schema>& schema = reader.GetSchema();
if (schema == nullptr) {
// TODO: Test and build the driver against a server that returns
// GH-46561 TODO: Test and build the driver against a server that returns
// complex types columns with the children
// types and handle the failure properly.
// https://github.com/apache/arrow/issues/46561
continue;
}
for (int i = 0; i < schema->num_fields(); ++i) {
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/get_info_cache.cc
Original file line number Diff line number Diff line change
Expand Up @@ -316,8 +316,8 @@ bool GetInfoCache::LoadInfoFromServer() {
std::string server_name(
reinterpret_cast<StringScalar*>(scalar->child_value().get())->view());

// TODO: Consider creating different properties in GetSqlInfo.
// TODO: Investigate if SQL_SERVER_NAME should just be the host
// GH-47855 TODO: Consider creating different properties in GetSqlInfo.
// GH-47856 TODO: Investigate if SQL_SERVER_NAME should just be the host
// address as well. In JDBC, FLIGHT_SQL_SERVER_NAME is only used for
// the DatabaseProductName.
info_[SQL_SERVER_NAME] = server_name;
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/json_converter.cc
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ class ScalarToJson : public ScalarVisitor {
}

Status Visit(const DurationScalar& scalar) override {
// TODO: Append TimeUnit on conversion
// GH-47857 TODO: Append TimeUnit on conversion
return ConvertScalarToStringAndWrite(scalar, writer_);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ TEST(ConvertToJson, MonthInterval) {
}

TEST(ConvertToJson, Duration) {
// TODO: Append TimeUnit on conversion
// GH-47857 TODO: Append TimeUnit on conversion
ASSERT_EQ("\"123\"", ConvertToJson(DurationScalar(123, TimeUnit::SECOND)));
ASSERT_EQ("\"123\"", ConvertToJson(DurationScalar(123, TimeUnit::MILLI)));
ASSERT_EQ("\"123\"", ConvertToJson(DurationScalar(123, TimeUnit::MICRO)));
Expand Down
4 changes: 2 additions & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_connection.cc
Original file line number Diff line number Diff line change
Expand Up @@ -233,8 +233,8 @@ SQLRETURN ODBCConnection::GetInfo(SQLUSMALLINT info_type, SQLPOINTER value,
case SQL_COLUMN_ALIAS:
case SQL_DBMS_NAME:
case SQL_DBMS_VER:
case SQL_DRIVER_NAME: // TODO: This should be the driver's filename and shouldn't
// come from the SPI.
case SQL_DRIVER_NAME: // GH-47858 TODO: This should be the driver's filename and
// shouldn't come from the SPI.
case SQL_DRIVER_VER:
case SQL_SEARCH_PATTERN_ESCAPE:
case SQL_SERVER_NAME:
Expand Down
10 changes: 6 additions & 4 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_descriptor.cc
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ void ODBCDescriptor::GetField(SQLSMALLINT record_number, SQLSMALLINT field_ident
throw DriverException("Invalid descriptor index", "07009");
}

// TODO: Restrict fields based on AppDescriptor IPD, and IRD.
// GH-47867 TODO: Restrict fields based on AppDescriptor IPD, and IRD.

bool length_in_bytes = true;
SQLSMALLINT zero_based_record = record_number - 1;
Expand Down Expand Up @@ -507,10 +507,12 @@ void ODBCDescriptor::PopulateFromResultSetMetadata(ResultSetMetadata* rsmd) {
rsmd->IsAutoUnique(one_based_index) ? SQL_TRUE : SQL_FALSE;
records_[i].case_sensitive =
rsmd->IsCaseSensitive(one_based_index) ? SQL_TRUE : SQL_FALSE;
records_[i].datetime_interval_precision; // TODO - update when rsmd adds this
records_[i].datetime_interval_precision; // GH-47869 TODO implement
// `SQL_DESC_DATETIME_INTERVAL_PRECISION`
SQLINTEGER num_prec_radix = rsmd->GetNumPrecRadix(one_based_index);
records_[i].num_prec_radix = num_prec_radix > 0 ? num_prec_radix : 0;
records_[i].datetime_interval_code; // TODO
records_[i].datetime_interval_code; // GH-47868 TODO implement
// `SQL_DESC_DATETIME_INTERVAL_CODE`
records_[i].fixed_prec_scale =
rsmd->IsFixedPrecScale(one_based_index) ? SQL_TRUE : SQL_FALSE;
records_[i].nullable = rsmd->IsNullable(one_based_index);
Expand Down Expand Up @@ -579,5 +581,5 @@ void ODBCDescriptor::SetDataPtrOnRecord(SQLPOINTER data_ptr, SQLSMALLINT record_
}

void DescriptorRecord::CheckConsistency() {
// TODO
// GH-47870 TODO implement
}
4 changes: 2 additions & 2 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,7 @@ bool ODBCStatement::Fetch(size_t rows, SQLULEN* row_count_ptr,
}

if (current_ard_->HaveBindingsChanged()) {
// TODO: Deal handle when offset != buffer_length.
// GH-47871 TODO: handle when offset != buffer_length.

// Wipe out all bindings in the ResultSet.
// Note that the number of ARD records can both be more or less
Expand Down Expand Up @@ -728,7 +728,7 @@ SQLRETURN ODBCStatement::GetData(SQLSMALLINT record_number, SQLSMALLINT c_type,

SQLSMALLINT evaluated_c_type = c_type;

// TODO: Get proper default precision and scale from abstraction.
// GH-47872 TODO: Get proper default precision and scale from abstraction.
int precision = 38; // arrow::Decimal128Type::kMaxPrecision;
int scale = 0;

Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/spi/statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ class Statement {
virtual ~Statement() = default;

/// \brief Statement attributes that can be called at anytime.
////TODO: Document attributes
/// GH-47850 TODO: Document attributes
enum StatementAttributeId {
MAX_LENGTH, // size_t - The maximum length when retrieving variable length data. 0
// means no limit.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ bool DsnConfigurationWindow::OnMessage(UINT msg, WPARAM wparam, LPARAM lparam) {
open_file_name.lpstrFile = file_name;
open_file_name.lpstrFile[0] = '\0';
open_file_name.nMaxFile = FILENAME_MAX;
// TODO: What type should this be?
// GH-47851 TODO: Update `lpstrFilter` to correct value
open_file_name.lpstrFilter = L"All\0*.*";
open_file_name.nFilterIndex = 1;
open_file_name.lpstrFileTitle = NULL;
Expand Down
1 change: 0 additions & 1 deletion cpp/src/arrow/flight/sql/odbc/odbc_impl/ui/window.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ HINSTANCE GetHInstance() {
TCHAR sz_file_name[MAX_PATH];
GetModuleFileName(NULL, sz_file_name, MAX_PATH);

// TODO: This needs to be the module name.
HINSTANCE h_instance = GetModuleHandle(sz_file_name);

if (h_instance == NULL) {
Expand Down
9 changes: 5 additions & 4 deletions cpp/src/arrow/flight/sql/odbc/odbc_impl/util.cc
Original file line number Diff line number Diff line change
Expand Up @@ -116,12 +116,13 @@ SqlDataType GetDataTypeFromArrowFieldV3(const std::shared_ptr<Field>& field,
case Type::TIME64:
return SqlDataType_TYPE_TIME;
case Type::INTERVAL_MONTHS:
return SqlDataType_INTERVAL_MONTH; // TODO: maybe
// SqlDataType_INTERVAL_YEAR_TO_MONTH
return SqlDataType_INTERVAL_MONTH; // GH-47873 TODO: check and update to
// SqlDataType_INTERVAL_YEAR_TO_MONTH if it is
// more appropriate
case Type::INTERVAL_DAY_TIME:
return SqlDataType_INTERVAL_DAY;

// TODO: Handle remaining types.
// GH-47873 TODO: Handle remaining types.
case Type::INTERVAL_MONTH_DAY_NANO:
case Type::LIST:
case Type::STRUCT:
Expand Down Expand Up @@ -669,7 +670,7 @@ optional<int32_t> GetDisplaySize(SqlDataType data_type,
case SqlDataType_INTERVAL_HOUR_TO_MINUTE:
case SqlDataType_INTERVAL_HOUR_TO_SECOND:
case SqlDataType_INTERVAL_MINUTE_TO_SECOND:
return nullopt; // TODO: Implement for INTERVAL types
return nullopt; // GH-47874 TODO: Implement for INTERVAL types
case SqlDataType_GUID:
return 36;
default:
Expand Down
2 changes: 1 addition & 1 deletion cpp/src/arrow/flight/sql/odbc/tests/odbc_test_suite.cc
Original file line number Diff line number Diff line change
Expand Up @@ -380,7 +380,7 @@ std::string GetOdbcErrorMessage(SQLSMALLINT handle_type, SQLHANDLE handle) {
return res;
}

// TODO: once RegisterDsn is implemented in Mac and Linux, the following can be
// GH-47822 TODO: once RegisterDsn is implemented in Mac and Linux, the following can be
// re-enabled.
#if defined _WIN32 || defined _WIN64
bool WriteDSN(std::string connection_str) {
Expand Down
Loading