Skip to content

Commit c2b53d0

Browse files
lawrinnlawrinn
committed
ODBC-474 Correction for the fix
The initial fix was in the commit "Fix of how parameter codec process insdicator values." 62010b2 It had a bug with moving indicator pointers to the next row and not restting NULL indicator value in the bind structure. Also this commit fixes error that column level colback would not move descriptor pointers if row was skipped or if NULL value indicator was set.
1 parent a3ced65 commit c2b53d0

File tree

4 files changed

+46
-25
lines changed

4 files changed

+46
-25
lines changed

‎driver/ma_codec.cpp‎

Lines changed: 27 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/************************************************************************************
2-
Copyright (C) 2024 MariaDB Corporation plc
2+
Copyright (C) 2024,2025 MariaDB Corporation plc
33
44
This library is free software; you can redistribute it and/or
55
modify it under the terms of the GNU Library General Public
@@ -53,20 +53,22 @@ namespace mariadb
5353
bind->u.indicator= &paramIndicatorIgnoreRow;
5454
return false;
5555
}
56-
else {
57-
bind->u.indicator= &paramIndicatorNone;
58-
}
5956
MADB_Stmt *stmt= static_cast<MADB_Stmt*>(data);
6057
for (auto i= 0; i < MADB_STMT_PARAM_COUNT(stmt); ++i) {
6158
auto indicator= indPtr[i];
59+
// Resetting to remove that has been left from the previous row
60+
(bind + i)->u.indicator= &paramIndicatorNone;
6261
if (indicator == nullptr) {
62+
// Technically, if there is parameter in query, but app hasn't bound it - we probably should use default
63+
// But if column does not have default this will yield error. Maybe NULL is safer. Also, need to check
64+
// if it's possible/allowed by specs not to have all parameters bound.
6365
//(bind + i)->u.indicator= &paramIndicatorDef;
6466
}
6567
else {
6668
if (*indicator == SQL_NULL_DATA) {
6769
(bind + i)->u.indicator= &paramIndicatorNull;
6870
}
69-
indPtr[i]= indicator + arrStep;
71+
indPtr[i]= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(indicator) + arrStep);
7072
}
7173
}
7274
return false;
@@ -75,17 +77,17 @@ namespace mariadb
7577

7678
bool FixedSizeCopyCodec::operator()(void * data, MYSQL_BIND * bind, uint32_t col_nr, uint32_t row_nr)
7779
{
80+
it.move(row_nr);
7881
bind->buffer= it.value();
79-
it.next();
8082
return false;
8183
}
8284

8385

8486
bool CopyCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
8587
{
88+
it.move(row_nr);
8689
bind->buffer= it.value();
8790
bind->buffer_length= getLength(*it.length(), bind->buffer);
88-
it.next();
8991
return false;
9092
}
9193

@@ -96,6 +98,7 @@ namespace mariadb
9698
SQLULEN mbLength=0;
9799
MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
98100

101+
it.move(row_nr);
99102
if (!it.length() || *it.length() == SQL_NTS)
100103
{
101104
/* CRec->OctetLength eq 0 means not 0-length buffer, but that this value is not specified. Thus -1, for SqlwcsLen
@@ -120,7 +123,6 @@ namespace mariadb
120123

121124
bind->buffer_length= (unsigned long)mbLength;
122125
bind->buffer= it.getDescRec()->InternalBuffer;
123-
it.next();
124126
return false;
125127
}
126128

@@ -134,9 +136,9 @@ namespace mariadb
134136

135137
bool BitCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
136138
{
139+
it.move(row_nr);
137140
bind->buffer= &buf;
138141
buf= MADB_ConvertCharToBit(reinterpret_cast<MADB_Stmt*>(data), static_cast<char*>(it.value()));
139-
it.next();
140142
return false;
141143
}
142144

@@ -152,14 +154,15 @@ namespace mariadb
152154
{
153155
MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
154156
bool isTime;
157+
158+
it.move(row_nr);
155159
MADB_Str2Ts(static_cast<char*>(it.value()), it.length() ? static_cast<std::size_t>(*it.length()) : 0, &buf,
156160
false, &Stmt->Error, &isTime);
157161
if (buf.second_part != 0)
158162
{
159163
MADB_SetError(&Stmt->Error, MADB_ERR_22008, nullptr, 0);
160164
return true;
161165
}
162-
it.next();
163166
return false;
164167
}
165168

@@ -175,14 +178,15 @@ namespace mariadb
175178
{
176179
MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
177180
bool isTime;
181+
182+
it.move(row_nr);
178183
MADB_Str2Ts(static_cast<char*>(it.value()), it.length() ? static_cast<std::size_t>(*it.length()) : 0, &buf,
179184
false, &Stmt->Error, &isTime);
180185
if (buf.hour || buf.minute || buf.second || buf.second_part)
181186
{
182187
MADB_SetError(&Stmt->Error, MADB_ERR_22008, nullptr, 0);
183188
return true;
184189
}
185-
it.next();
186190
return false;
187191
}
188192

@@ -198,14 +202,15 @@ namespace mariadb
198202
{
199203
MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
200204
bool isTime= false;
205+
206+
it.move(row_nr);
201207
MADB_Str2Ts(static_cast<char*>(it.value()), it.length() ? static_cast<std::size_t>(*it.length()) : 0, &buf,
202208
false, &Stmt->Error, &isTime);
203209
if ((!isTime && buf.year == 0) || buf.month == 0 || buf.day == 0)
204210
{
205211
MADB_SetError(&Stmt->Error, MADB_ERR_22018, nullptr, 0);
206212
return true;
207213
}
208-
it.next();
209214
return false;
210215
}
211216

@@ -223,6 +228,7 @@ namespace mariadb
223228
int32_t errorCode= 0;
224229
SQL_NUMERIC_STRUCT *p;
225230

231+
it.move(row_nr);
226232
p= reinterpret_cast<SQL_NUMERIC_STRUCT*>(it.value());
227233
p->scale= scale;
228234
p->precision= precision;
@@ -233,7 +239,6 @@ namespace mariadb
233239
MADB_SetError(&Stmt->Error, errorCode, nullptr, 0);
234240
return true;
235241
}
236-
it.next();
237242
return false;
238243
}
239244

@@ -249,6 +254,7 @@ namespace mariadb
249254

250255
bool Ts2DateCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
251256
{
257+
it.move(row_nr);
252258
SQL_TIMESTAMP_STRUCT *ts= reinterpret_cast<SQL_TIMESTAMP_STRUCT*>(it.value());
253259
if (ts->hour || ts->minute || ts->second || ts->fraction)
254260
{
@@ -259,7 +265,7 @@ namespace mariadb
259265
buf.year= ts->year;
260266
buf.month= ts->month;
261267
buf.day= ts->day;
262-
it.next();
268+
it.move(row_nr);
263269
return false;
264270
}
265271

@@ -275,6 +281,7 @@ namespace mariadb
275281

276282
bool Ts2TimeCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
277283
{
284+
it.move(row_nr);
278285
SQL_TIMESTAMP_STRUCT *ts= reinterpret_cast<SQL_TIMESTAMP_STRUCT*>(it.value());
279286
MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
280287

@@ -291,7 +298,6 @@ namespace mariadb
291298
buf.hour= ts->hour;
292299
buf.minute= ts->minute;
293300
buf.second= ts->second;
294-
it.next();
295301
return false;
296302
}
297303

@@ -307,10 +313,11 @@ namespace mariadb
307313

308314
bool TsCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
309315
{
316+
it.move(row_nr);
310317
SQL_TIMESTAMP_STRUCT *ts= reinterpret_cast<SQL_TIMESTAMP_STRUCT*>(it.value());
311318

312319
MADB_CopyOdbcTsToMadbTime(ts, &buf);
313-
it.next();
320+
314321
return false;
315322
}
316323

@@ -353,6 +360,7 @@ namespace mariadb
353360

354361
bool Time2TsCodec::operator()(void *data, MYSQL_BIND * bind, uint32_t col_nr, uint32_t row_nr)
355362
{
363+
it.move(row_nr);
356364
SQL_TIME_STRUCT *ts= reinterpret_cast<SQL_TIME_STRUCT*>(it.value());
357365
if (checkValidTime && !VALID_TIME(ts)) {
358366
MADB_Stmt *Stmt= reinterpret_cast<MADB_Stmt*>(data);
@@ -362,7 +370,6 @@ namespace mariadb
362370
buf.hour= ts->hour;
363371
buf.minute= ts->minute;
364372
buf.second= ts->second;
365-
it.next();
366373
return false;
367374
}
368375

@@ -382,6 +389,7 @@ namespace mariadb
382389

383390
bool IntrervalHmsCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
384391
{
392+
it.move(row_nr);
385393
SQL_INTERVAL_STRUCT *is= reinterpret_cast<SQL_INTERVAL_STRUCT*>(it.value());
386394
buf.hour= is->intval.day_second.hour;
387395
buf.minute= is->intval.day_second.minute;
@@ -390,7 +398,7 @@ namespace mariadb
390398
}
391399

392400
buf.second_part= 0;
393-
it.next();
401+
394402
return false;
395403
}
396404

@@ -407,11 +415,11 @@ namespace mariadb
407415
/* {{{ DateCodec::operator() */
408416
bool DateCodec::operator()(void *data, MYSQL_BIND *bind, uint32_t col_nr, uint32_t row_nr)
409417
{
418+
it.move(row_nr);
410419
SQL_DATE_STRUCT *ts= reinterpret_cast<SQL_DATE_STRUCT*>(it.value());
411420
buf.year= ts->year;
412421
buf.month= ts->month;
413422
buf.day= ts->day;
414-
it.next();
415423
return true;
416424
}
417425
/* }}} */

‎driver/ma_desc.h‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@ class DescArrayIterator
7373
SQLLEN* octetLengthPtr= nullptr;
7474
SQLLEN* indicatorPtr= nullptr;
7575
std::size_t lengthOffset= sizeof(SQLLEN);
76+
std::size_t position= 0;
7677

7778
public:
7879
DescArrayIterator(MADB_Header& header, MADB_DescRecord& rec, SQLSMALLINT i);
@@ -83,8 +84,19 @@ class DescArrayIterator
8384
indicatorPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(indicatorPtr) + lengthOffset);
8485
}
8586
octetLengthPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(octetLengthPtr) + lengthOffset);
87+
++position;
8688
return (valuePtr= (void*)((char*)valuePtr + valueOffset));
8789
}
90+
inline void* move(std::size_t pos) {
91+
int64_t offset= (static_cast<int64_t>(pos) - position)*lengthOffset;
92+
if (indicatorPtr != octetLengthPtr) {
93+
indicatorPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(indicatorPtr) + offset);
94+
}
95+
octetLengthPtr= reinterpret_cast<SQLLEN*>(reinterpret_cast<char*>(octetLengthPtr) + offset);
96+
offset= (static_cast<int64_t>(pos) - position)*valueOffset;
97+
position= pos;
98+
return (valuePtr= (void*)((char*)valuePtr + offset));
99+
}
88100
inline void* value() { return valuePtr; }
89101
inline SQLLEN* length() { return octetLengthPtr; }
90102
inline SQLLEN* indicator(){ return indicatorPtr; }

‎test/bulk.c‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -813,15 +813,16 @@ int main(int argc, char **argv)
813813
{
814814
int tests= sizeof(my_tests)/sizeof(MA_ODBC_TESTS) - 1;
815815
int result;
816-
char setpcallback[256];
817-
sprintf(setpcallback, "%s;PCALLBACK=0", add_connstr);
818-
add_connstr= setpcallback;
816+
char setpcallback[256], *original;
819817
get_options(argc, argv);
818+
original= add_connstr;
819+
sprintf(setpcallback, "%s;PCALLBACK=0", add_connstr);
820+
storedAddConnstr= add_connstr= setpcallback;
820821
plan(tests);
821822
mark_all_tests_normal(my_tests);
822823
result= run_tests(my_tests);
823824

824-
sprintf(setpcallback, "%s;PCALLBACK=1", add_connstr);
825-
add_connstr= setpcallback;
825+
sprintf(setpcallback, "%s;PCALLBACK=1", original);
826+
storedAddConnstr= add_connstr= setpcallback;
826827
return run_tests(my_tests) || result;
827828
}

‎test/tap.h‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1222,7 +1222,7 @@ int run_tests_ex(MA_ODBC_TESTS *tests, BOOL ProvideWConnection)
12221222
SQLAllocHandle(SQL_HANDLE_STMT, wConnection, &wStmt);
12231223
}
12241224

1225-
/* Relieving tests from resoting my_options and/or add_connstr. Also, a test may fail */
1225+
/* Relieving tests from restoring my_options and/or add_connstr. Also, a test may fail */
12261226
my_options= defaultOptions;
12271227
add_connstr= storedAddConnstr;
12281228
/* reset Statement */

0 commit comments

Comments
 (0)
⚡