This is an automated email from the ASF dual-hosted git repository.
lidavidm pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/arrow.git
The following commit(s) were added to refs/heads/main by this push:
new ff3ca553c7 GH-48707: [C++][FlightRPC] Use IRD precision/scale defaults
with ARD override in SQLGetData (#48708)
ff3ca553c7 is described below
commit ff3ca553c7312224c81d94907439b2b25ddccdc2
Author: Hyukjin Kwon <[email protected]>
AuthorDate: Fri Jan 9 10:16:49 2026 +0900
GH-48707: [C++][FlightRPC] Use IRD precision/scale defaults with ARD
override in SQLGetData (#48708)
### Rationale for this change
https://github.com/apache/arrow/blob/8fc54a35f7df672d416ebd12a9558d320fba9afe/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc#L746-L748
This was introduced by
https://github.com/apache/arrow/commit/ed36107ad869dd0db5c33a7c3e5484f66a461a4f
The `GetData` function was using hardcoded precision (38) and scale (0)
values instead of retrieving them from the IRD (Implementation Row Descriptor).
This fix ensures that precision/scale are properly retrieved from the IRD as
defaults, and can be overridden by ARD (Application Row Descriptor) values when
specified, following ODBC specification behavior.
### What changes are included in this PR?
- Modified `ODBCStatement::GetData()` to use IRD precision/scale as
defaults instead of hardcoded values
- ARD precision/scale now properly override IRD values when `SQL_ARD_TYPE`
or `SQL_C_DEFAULT` is used
- Added unit tests to verify IRD defaults and ARD override behavior
### Are these changes tested?
Yes. Added two tests in `statement_test.cc`:
- `TestGetDataPrecisionScaleUsesIRDAsDefault`: Verifies IRD precision/scale
are used as defaults
- `TestGetDataPrecisionScaleUsesARDWhenSet`: Verifies ARD precision/scale
override IRD when set
### Are there any user-facing changes?
I think it's fair to say a no. This is an internal fix that ensures correct
ODBC behavior.
* GitHub Issue: #48707
Authored-by: Hyukjin Kwon <[email protected]>
Signed-off-by: David Li <[email protected]>
---
.../flight/sql/odbc/odbc_impl/odbc_statement.cc | 11 ++-
.../arrow/flight/sql/odbc/tests/statement_test.cc | 96 ++++++++++++++++++++++
2 files changed, 103 insertions(+), 4 deletions(-)
diff --git a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc
b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc
index a80ff7d61c..159c58102c 100644
--- a/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc
+++ b/cpp/src/arrow/flight/sql/odbc/odbc_impl/odbc_statement.cc
@@ -25,6 +25,7 @@
#include "arrow/flight/sql/odbc/odbc_impl/spi/result_set_metadata.h"
#include "arrow/flight/sql/odbc/odbc_impl/spi/statement.h"
#include "arrow/flight/sql/odbc/odbc_impl/types.h"
+#include "arrow/type.h"
#include <sql.h>
#include <sqlext.h>
@@ -743,9 +744,12 @@ SQLRETURN ODBCStatement::GetData(SQLSMALLINT
record_number, SQLSMALLINT c_type,
SQLSMALLINT evaluated_c_type = c_type;
- // TODO: Get proper default precision and scale from abstraction.
- int precision = 38; // arrow::Decimal128Type::kMaxPrecision;
- int scale = 0;
+ // Get precision and scale from IRD (implementation row descriptor) as
defaults.
+ // These can be overridden by ARD (application row descriptor) if specified.
+ const DescriptorRecord& ird_record = ird_->GetRecords()[record_number - 1];
+ int precision =
+ ird_record.precision > 0 ? ird_record.precision :
Decimal128Type::kMaxPrecision;
+ int scale = ird_record.scale;
if (c_type == SQL_ARD_TYPE) {
if (record_number > current_ard_->GetRecords().size()) {
@@ -767,7 +771,6 @@ SQLRETURN ODBCStatement::GetData(SQLSMALLINT record_number,
SQLSMALLINT c_type,
scale = ard_record.scale;
}
- const DescriptorRecord& ird_record = ird_->GetRecords()[record_number - 1];
evaluated_c_type = getc_typeForSQLType(ird_record);
}
diff --git a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc
b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc
index 1ce07c89f1..1a38bd0e24 100644
--- a/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc
+++ b/cpp/src/arrow/flight/sql/odbc/tests/statement_test.cc
@@ -646,6 +646,102 @@ TEST_F(StatementRemoteTest,
TestSQLExecDirectVarbinaryQueryDefaultType) {
EXPECT_EQ('\xEF', varbinary_val[2]);
}
+// TODO(GH-48730): Enable this test when ARD/IRD descriptor support is fully
implemented
+TYPED_TEST(StatementTest, DISABLED_TestGetDataPrecisionScaleUsesIRDAsDefault) {
+ // Verify that SQLGetData uses IRD precision/scale as defaults when ARD
values are unset
+ std::wstring wsql = L"SELECT CAST('123.45' AS NUMERIC) as decimal_col;";
+ std::vector<SQLWCHAR> sql(wsql.begin(), wsql.end());
+
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLExecDirect(this->stmt, &sql[0],
static_cast<SQLINTEGER>(sql.size())));
+
+ ASSERT_EQ(SQL_SUCCESS, SQLFetch(this->stmt));
+
+ // Get precision and scale from IRD
+ SQLLEN ird_precision = 0;
+ SQLLEN ird_scale = 0;
+ SQLHDESC ird = nullptr;
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLGetStmtAttr(this->stmt, SQL_ATTR_IMP_ROW_DESC, &ird, 0,
nullptr));
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLGetDescField(ird, 1, SQL_DESC_PRECISION, &ird_precision, 0,
nullptr));
+ ASSERT_EQ(SQL_SUCCESS, SQLGetDescField(ird, 1, SQL_DESC_SCALE, &ird_scale,
0, nullptr));
+
+ // Test with SQL_C_NUMERIC - should use IRD precision/scale
+ SQL_NUMERIC_STRUCT numeric_val;
+ memset(&numeric_val, 0, sizeof(numeric_val));
+ SQLLEN indicator;
+ ASSERT_EQ(SQL_SUCCESS, SQLGetData(this->stmt, 1, SQL_C_NUMERIC, &numeric_val,
+ sizeof(SQL_NUMERIC_STRUCT), &indicator));
+ EXPECT_EQ(static_cast<SQLSMALLINT>(ird_precision), numeric_val.precision);
+ EXPECT_EQ(static_cast<SQLSMALLINT>(ird_scale), numeric_val.scale);
+
+ // Test with SQL_C_DEFAULT when ARD is unset (0) - should fall back to IRD
+ // precision/scale
+ SQLHDESC ard = nullptr;
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_ROW_DESC, &ard, 0,
nullptr));
+ ASSERT_EQ(SQL_SUCCESS, SQLSetDescField(ard, 1, SQL_DESC_PRECISION,
+ reinterpret_cast<SQLPOINTER>(0), 0));
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLSetDescField(ard, 1, SQL_DESC_SCALE,
reinterpret_cast<SQLPOINTER>(0), 0));
+
+ memset(&numeric_val, 0, sizeof(numeric_val));
+ ASSERT_EQ(SQL_SUCCESS, SQLGetData(this->stmt, 1, SQL_C_DEFAULT, &numeric_val,
+ sizeof(SQL_NUMERIC_STRUCT), &indicator));
+ EXPECT_EQ(static_cast<SQLSMALLINT>(ird_precision), numeric_val.precision);
+ EXPECT_EQ(static_cast<SQLSMALLINT>(ird_scale), numeric_val.scale);
+}
+
+// TODO(GH-48730): Enable this test when ARD/IRD descriptor support is fully
implemented
+TYPED_TEST(StatementTest, DISABLED_TestGetDataPrecisionScaleUsesARDWhenSet) {
+ // Verify that SQLGetData uses ARD precision/scale when set, for both
SQL_ARD_TYPE and
+ // SQL_C_DEFAULT
+ std::wstring wsql = L"SELECT CAST('123.45' AS NUMERIC) as decimal_col;";
+ std::vector<SQLWCHAR> sql(wsql.begin(), wsql.end());
+
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLExecDirect(this->stmt, &sql[0],
static_cast<SQLINTEGER>(sql.size())));
+
+ ASSERT_EQ(SQL_SUCCESS, SQLFetch(this->stmt));
+
+ SQLHDESC ard = nullptr;
+ ASSERT_EQ(SQL_SUCCESS,
+ SQLGetStmtAttr(this->stmt, SQL_ATTR_APP_ROW_DESC, &ard, 0,
nullptr));
+
+ // Test with SQL_ARD_TYPE
+ SQLSMALLINT ard_precision = 15;
+ SQLSMALLINT ard_scale = 3;
+ ASSERT_EQ(SQL_SUCCESS, SQLSetDescField(ard, 1, SQL_DESC_PRECISION,
+
reinterpret_cast<SQLPOINTER>(ard_precision), 0));
+ ASSERT_EQ(SQL_SUCCESS, SQLSetDescField(ard, 1, SQL_DESC_SCALE,
+
reinterpret_cast<SQLPOINTER>(ard_scale), 0));
+ ASSERT_EQ(SQL_SUCCESS, SQLSetDescField(ard, 1, SQL_DESC_TYPE,
+
reinterpret_cast<SQLPOINTER>(SQL_C_NUMERIC), 0));
+
+ SQL_NUMERIC_STRUCT numeric_val;
+ memset(&numeric_val, 0, sizeof(numeric_val));
+ SQLLEN indicator;
+ ASSERT_EQ(SQL_SUCCESS, SQLGetData(this->stmt, 1, SQL_ARD_TYPE, &numeric_val,
+ sizeof(SQL_NUMERIC_STRUCT), &indicator));
+ EXPECT_EQ(ard_precision, numeric_val.precision);
+ EXPECT_EQ(ard_scale, numeric_val.scale);
+
+ // Test with SQL_C_DEFAULT
+ ard_precision = 20;
+ ard_scale = 4;
+ ASSERT_EQ(SQL_SUCCESS, SQLSetDescField(ard, 1, SQL_DESC_PRECISION,
+
reinterpret_cast<SQLPOINTER>(ard_precision), 0));
+ ASSERT_EQ(SQL_SUCCESS, SQLSetDescField(ard, 1, SQL_DESC_SCALE,
+
reinterpret_cast<SQLPOINTER>(ard_scale), 0));
+
+ memset(&numeric_val, 0, sizeof(numeric_val));
+ ASSERT_EQ(SQL_SUCCESS, SQLGetData(this->stmt, 1, SQL_C_DEFAULT, &numeric_val,
+ sizeof(SQL_NUMERIC_STRUCT), &indicator));
+ EXPECT_EQ(ard_precision, numeric_val.precision);
+ EXPECT_EQ(ard_scale, numeric_val.scale);
+}
+
TYPED_TEST(StatementTest, TestSQLExecDirectGuidQueryUnsupported) {
// Query GUID as string as SQLite does not support GUID
std::wstring wsql = L"SELECT 'C77313CF-4E08-47CE-B6DF-94DD2FCF3541' AS
guid;";
