Skip to content

Commit 2964c54

Browse files
Mandukhai-AlimaaMandukhai-Alimaa
authored
fix(c/driver/postgresql): handle empty strings correctly in parameter binding (#3601)
This PR fixes an issue in the PostgreSQL driver’s parameter binding logic where empty strings were incorrectly treated as NULL values. Null detection was inferred from param_lengths[col] == 0 and empty strings (valid zero-length values) were misclassified as NULL. Closes #3585.
1 parent 11ed854 commit 2964c54

File tree

2 files changed

+102
-2
lines changed

2 files changed

+102
-2
lines changed

‎c/driver/postgresql/bind_stream.h‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -201,9 +201,11 @@ struct BindStream {
201201
int result_format) {
202202
param_buffer->size_bytes = 0;
203203
int64_t last_offset = 0;
204+
std::vector<bool> is_null_param(array_view->n_children);
204205

205206
for (int64_t col = 0; col < array_view->n_children; col++) {
206-
if (!ArrowArrayViewIsNull(array_view->children[col], current_row)) {
207+
is_null_param[col] = ArrowArrayViewIsNull(array_view->children[col], current_row);
208+
if (!is_null_param[col]) {
207209
// Note that this Write() call currently writes the (int32_t) byte size of the
208210
// field in addition to the serialized value.
209211
UNWRAP_NANOARROW(
@@ -225,7 +227,7 @@ struct BindStream {
225227
last_offset = 0;
226228
for (int64_t col = 0; col < array_view->n_children; col++) {
227229
last_offset += sizeof(int32_t);
228-
if (param_lengths[col] == 0) {
230+
if (is_null_param[col]) {
229231
param_values[col] = nullptr;
230232
} else {
231233
param_values[col] = reinterpret_cast<char*>(param_buffer->data) + last_offset;

‎c/driver/postgresql/postgresql_test.cc‎

Lines changed: 98 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1712,6 +1712,104 @@ TEST_F(PostgresStatementTest, ExecuteParameterizedQueryWithRowsAffected) {
17121712
}
17131713
}
17141714

1715+
// Test for making sure empty string/binary parameters are inserted correct
1716+
TEST_F(PostgresStatementTest, EmptyStringAndBinaryParameter) {
1717+
ASSERT_THAT(quirks()->DropTable(&connection, "adbc_test", &error), IsOkStatus(&error));
1718+
ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error));
1719+
1720+
// Create test table with both TEXT and BYTEA columns
1721+
{
1722+
ASSERT_THAT(AdbcStatementSetSqlQuery(
1723+
&statement,
1724+
"CREATE TABLE adbc_test (text_data TEXT, binary_data BYTEA)", &error),
1725+
IsOkStatus(&error));
1726+
adbc_validation::StreamReader reader;
1727+
ASSERT_THAT(
1728+
AdbcStatementExecuteQuery(&statement, &reader.stream.value, nullptr, &error),
1729+
IsOkStatus(&error));
1730+
ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
1731+
ASSERT_NO_FATAL_FAILURE(reader.Next());
1732+
ASSERT_EQ(reader.array->release, nullptr);
1733+
}
1734+
1735+
// Insert empty string and binary via parameters
1736+
{
1737+
nanoarrow::UniqueSchema schema_bind;
1738+
ArrowSchemaInit(schema_bind.get());
1739+
ASSERT_THAT(ArrowSchemaSetTypeStruct(schema_bind.get(), 2),
1740+
adbc_validation::IsOkErrno());
1741+
ASSERT_THAT(ArrowSchemaSetType(schema_bind->children[0], NANOARROW_TYPE_STRING),
1742+
adbc_validation::IsOkErrno());
1743+
ASSERT_THAT(ArrowSchemaSetType(schema_bind->children[1], NANOARROW_TYPE_BINARY),
1744+
adbc_validation::IsOkErrno());
1745+
1746+
nanoarrow::UniqueArray bind;
1747+
ASSERT_THAT(ArrowArrayInitFromSchema(bind.get(), schema_bind.get(), nullptr),
1748+
adbc_validation::IsOkErrno());
1749+
ASSERT_THAT(ArrowArrayStartAppending(bind.get()), adbc_validation::IsOkErrno());
1750+
1751+
// Add one row with empty string and empty binary parameters
1752+
ASSERT_THAT(ArrowArrayAppendString(bind->children[0], ArrowCharView("")),
1753+
adbc_validation::IsOkErrno());
1754+
ArrowBufferView empty_buffer = {{nullptr}, 0};
1755+
ASSERT_THAT(ArrowArrayAppendBytes(bind->children[1], empty_buffer),
1756+
adbc_validation::IsOkErrno());
1757+
ASSERT_THAT(ArrowArrayFinishElement(bind.get()), adbc_validation::IsOkErrno());
1758+
ASSERT_THAT(ArrowArrayFinishBuildingDefault(bind.get(), nullptr),
1759+
adbc_validation::IsOkErrno());
1760+
1761+
ASSERT_THAT(AdbcStatementSetSqlQuery(&statement,
1762+
"INSERT INTO adbc_test VALUES ($1, $2)", &error),
1763+
IsOkStatus(&error));
1764+
ASSERT_THAT(AdbcStatementBind(&statement, bind.get(), schema_bind.get(), &error),
1765+
IsOkStatus(&error));
1766+
1767+
adbc_validation::StreamReader reader;
1768+
ASSERT_THAT(
1769+
AdbcStatementExecuteQuery(&statement, &reader.stream.value, nullptr, &error),
1770+
IsOkStatus(&error));
1771+
ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
1772+
ASSERT_NO_FATAL_FAILURE(reader.Next());
1773+
ASSERT_EQ(reader.array->release, nullptr);
1774+
}
1775+
1776+
// Verify empty values were inserted correctly (not as NULL)
1777+
{
1778+
ASSERT_THAT(AdbcStatementSetSqlQuery(
1779+
&statement, "SELECT text_data, binary_data FROM adbc_test", &error),
1780+
IsOkStatus(&error));
1781+
adbc_validation::StreamReader reader;
1782+
ASSERT_THAT(
1783+
AdbcStatementExecuteQuery(&statement, &reader.stream.value, nullptr, &error),
1784+
IsOkStatus(&error));
1785+
ASSERT_NO_FATAL_FAILURE(reader.GetSchema());
1786+
ASSERT_NO_FATAL_FAILURE(reader.Next());
1787+
ASSERT_NE(reader.array->release, nullptr);
1788+
ASSERT_EQ(reader.array->length, 1);
1789+
1790+
// Row should contain empty values, not NULL
1791+
ASSERT_EQ(reader.array->children[0]->null_count, 0); // text_data
1792+
ASSERT_EQ(reader.array->children[1]->null_count, 0); // binary_data
1793+
1794+
// Check that both values are empty (string and binary)
1795+
// Check the single row
1796+
ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[0], 0));
1797+
struct ArrowBufferView string_view =
1798+
ArrowArrayViewGetBytesUnsafe(reader.array_view->children[0], 0);
1799+
ASSERT_EQ(string_view.size_bytes, 0); // Empty string should have size 0
1800+
1801+
ASSERT_FALSE(ArrowArrayViewIsNull(reader.array_view->children[1], 0));
1802+
struct ArrowBufferView binary_view =
1803+
ArrowArrayViewGetBytesUnsafe(reader.array_view->children[1], 0);
1804+
ASSERT_EQ(binary_view.size_bytes, 0); // Empty binary should have size 0
1805+
1806+
ASSERT_NO_FATAL_FAILURE(reader.Next());
1807+
ASSERT_EQ(reader.array->release, nullptr);
1808+
}
1809+
1810+
ASSERT_THAT(AdbcStatementRelease(&statement, &error), IsOkStatus(&error));
1811+
}
1812+
17151813
TEST_F(PostgresStatementTest, SqlExecuteCopyZeroRowOutputError) {
17161814
ASSERT_THAT(quirks()->DropTable(&connection, "adbc_test", &error), IsOkStatus(&error));
17171815
ASSERT_THAT(AdbcStatementNew(&connection, &statement, &error), IsOkStatus(&error));

0 commit comments

Comments
 (0)