Skip to content

Commit bd0bbeb

Browse files
Fix multi-row TVP SELECT returning last row (#4817)
Bug introduced in c497926 uses currentColumn which is being reset to 0 and creating an ordering issue in insert stmt for the TVP datatype. With this we want to fix the param ordering for the insertion. Issues Resolved BABEL-6511 Sign-off-by: Kushaal Shroff <kushaal@amazon.com>
1 parent 2d77741 commit bd0bbeb

5 files changed

Lines changed: 87 additions & 54 deletions

File tree

contrib/babelfishpg_tds/src/backend/tds/tdstypeio.c

Lines changed: 51 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
#include "src/include/tds_typeio.h"
4545
#include "src/include/err_handler.h"
4646
#include "src/include/tds_instr.h"
47+
#include "common/int.h"
4748

4849
#include "tds_data_map.c" /* include tables that used to initialize
4950
* hashmaps */
@@ -2120,26 +2121,14 @@ FetchTvpTypeOid(const ParameterToken token, char *tvpName)
21202121
char *query;
21212122

21222123
if ((rc = SPI_connect()) < 0)
2123-
{
2124-
/* Reset dialect. */
2125-
set_config_option("babelfishpg_tsql.sql_dialect", "tsql",
2126-
GUC_CONTEXT_CONFIG,
2127-
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
21282124
elog(ERROR, "SPI_connect() failed in TDS Listener "
21292125
"with return code %d", rc);
2130-
}
21312126

21322127
query = psprintf("SELECT '%s'::regtype::oid", tvpName);
21332128

21342129
rc = SPI_execute(query, false, 1);
21352130
if (rc != SPI_OK_SELECT)
2136-
{
2137-
/* Reset dialect. */
2138-
set_config_option("babelfishpg_tsql.sql_dialect", "tsql",
2139-
GUC_CONTEXT_CONFIG,
2140-
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
2141-
elog(ERROR, "Failed to insert in the underlying table for table-valued parameter: %d", rc);
2142-
}
2131+
elog(ERROR, "Failed to fetch TVP type OID: %d", rc);
21432132
tupdesc = SPI_tuptable->tupdesc;
21442133
row = SPI_tuptable->vals[0];
21452134

@@ -2217,6 +2206,9 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
22172206
GUC_CONTEXT_CONFIG,
22182207
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
22192208

2209+
PG_TRY();
2210+
{
2211+
22202212
if (!xactStarted)
22212213
StartTransactionCommand();
22222214
PushActiveSnapshot(GetTransactionSnapshot());
@@ -2238,10 +2230,6 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
22382230

22392231
if (rc != SPI_OK_UTILITY)
22402232
{
2241-
/* Reset dialect. */
2242-
set_config_option("babelfishpg_tsql.sql_dialect", "tsql",
2243-
GUC_CONTEXT_CONFIG,
2244-
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
22452233
elog(ERROR, "Failed to create the underlying table for table-valued parameter: %d", rc);
22462234
}
22472235

@@ -2252,10 +2240,25 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
22522240

22532241
{
22542242
char *src;
2255-
int nargs = token->tvpInfo->colCount * token->tvpInfo->rowCount;
2256-
Datum *values = palloc(nargs * sizeof(Datum));
2257-
char *nulls = palloc(nargs * sizeof(char));
2258-
Oid *argtypes = palloc(nargs * sizeof(Datum));
2243+
int nargs;
2244+
Datum *values;
2245+
char *nulls;
2246+
Oid *argtypes;
2247+
int paramIndex = 0;
2248+
2249+
/*
2250+
* pg_mul_s32_overflow guards colCount * rowCount, ensuring nargs
2251+
* fits in int32. Since nargs <= INT32_MAX, the subsequent
2252+
* palloc_array allocations cannot overflow on 64-bit systems.
2253+
*/
2254+
if (pg_mul_s32_overflow(token->tvpInfo->colCount, token->tvpInfo->rowCount, &nargs))
2255+
ereport(ERROR,
2256+
(errcode(ERRCODE_PROTOCOL_VIOLATION),
2257+
errmsg("TVP row/column count too large")));
2258+
2259+
values = palloc_array(Datum, nargs);
2260+
nulls = palloc_array(char, nargs);
2261+
argtypes = palloc_array(Oid, nargs);
22592262

22602263
query = " ";
22612264

@@ -2274,67 +2277,67 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
22742277
{
22752278
temp = &(row->columnValues[currentColumn]);
22762279
tempFuncInfo = TdsLookupTypeFunctionsByTdsId(colMetaData[currentColumn].columnTdsType, colMetaData[currentColumn].maxLen);
2277-
GetPgOid(argtypes[currentColumn], tempFuncInfo);
2278-
if (row->isNull[currentColumn] == 'n')
2279-
nulls[currentColumn] = row->isNull[currentColumn];
2280-
else
2280+
GetPgOid(argtypes[paramIndex], tempFuncInfo);
2281+
2282+
if (row->isNull[currentColumn] != 'n')
22812283
switch (colMetaData[currentColumn].columnTdsType)
22822284
{
22832285
case TDS_TYPE_CHAR:
22842286
case TDS_TYPE_VARCHAR:
2285-
values[currentColumn] = TdsTypeVarcharToDatum(temp, colMetaData[currentColumn].encoding, colMetaData[currentColumn].columnTdsType);
2287+
values[paramIndex] = TdsTypeVarcharToDatum(temp, colMetaData[currentColumn].encoding, colMetaData[currentColumn].columnTdsType);
22862288
break;
22872289
case TDS_TYPE_NCHAR:
22882290
case TDS_TYPE_NVARCHAR:
2289-
values[currentColumn] = TdsTypeNCharToDatum(temp);
2291+
values[paramIndex] = TdsTypeNCharToDatum(temp);
22902292
break;
22912293
case TDS_TYPE_INTEGER:
22922294
case TDS_TYPE_BIT:
2293-
values[currentColumn] = TdsTypeIntegerToDatum(temp, colMetaData[currentColumn].maxLen);
2295+
values[paramIndex] = TdsTypeIntegerToDatum(temp, colMetaData[currentColumn].maxLen);
22942296
break;
22952297
case TDS_TYPE_FLOAT:
2296-
values[currentColumn] = TdsTypeFloatToDatum(temp, colMetaData[currentColumn].maxLen);
2298+
values[paramIndex] = TdsTypeFloatToDatum(temp, colMetaData[currentColumn].maxLen);
22972299
break;
22982300
case TDS_TYPE_NUMERICN:
22992301
case TDS_TYPE_DECIMALN:
2300-
values[currentColumn] = TdsTypeNumericToDatum(temp, colMetaData[currentColumn].scale);
2302+
values[paramIndex] = TdsTypeNumericToDatum(temp, colMetaData[currentColumn].scale);
23012303
break;
23022304
case TDS_TYPE_VARBINARY:
23032305
case TDS_TYPE_BINARY:
2304-
values[currentColumn] = TdsTypeVarbinaryToDatum(temp);
2305-
argtypes[currentColumn] = tempFuncInfo->ttmtypeid;
2306+
values[paramIndex] = TdsTypeVarbinaryToDatum(temp);
2307+
argtypes[paramIndex] = tempFuncInfo->ttmtypeid;
23062308
break;
23072309
case TDS_TYPE_DATE:
2308-
values[currentColumn] = TdsTypeDateToDatum(temp);
2310+
values[paramIndex] = TdsTypeDateToDatum(temp);
23092311
break;
23102312
case TDS_TYPE_TIME:
2311-
values[currentColumn] = TdsTypeTimeToDatum(temp, colMetaData[currentColumn].scale, temp->len);
2313+
values[paramIndex] = TdsTypeTimeToDatum(temp, colMetaData[currentColumn].scale, temp->len);
23122314
break;
23132315
case TDS_TYPE_DATETIMEOFFSET:
2314-
values[currentColumn] = TdsTypeDatetimeoffsetToDatum(temp, colMetaData[currentColumn].scale, temp->len);
2316+
values[paramIndex] = TdsTypeDatetimeoffsetToDatum(temp, colMetaData[currentColumn].scale, temp->len);
23152317
break;
23162318
case TDS_TYPE_DATETIME2:
2317-
values[currentColumn] = TdsTypeDatetime2ToDatum(temp, colMetaData[currentColumn].scale, temp->len);
2319+
values[paramIndex] = TdsTypeDatetime2ToDatum(temp, colMetaData[currentColumn].scale, temp->len);
23182320
break;
23192321
case TDS_TYPE_DATETIMEN:
2320-
values[currentColumn] = TdsTypeDatetimeToDatum(temp);
2322+
values[paramIndex] = TdsTypeDatetimeToDatum(temp);
23212323
break;
23222324
case TDS_TYPE_MONEYN:
2323-
values[currentColumn] = TdsTypeMoneyToDatum(temp);
2325+
values[paramIndex] = TdsTypeMoneyToDatum(temp);
23242326
break;
23252327
case TDS_TYPE_XML:
2326-
values[currentColumn] = TdsTypeXMLToDatum(temp);
2328+
values[paramIndex] = TdsTypeXMLToDatum(temp);
23272329
break;
23282330
case TDS_TYPE_UNIQUEIDENTIFIER:
2329-
values[currentColumn] = TdsTypeUIDToDatum(temp);
2331+
values[paramIndex] = TdsTypeUIDToDatum(temp);
23302332
break;
23312333
case TDS_TYPE_SQLVARIANT:
2332-
values[currentColumn] = TdsTypeSqlVariantToDatum(temp);
2334+
values[paramIndex] = TdsTypeSqlVariantToDatum(temp);
23332335
break;
23342336
}
23352337
/* Build a string for bind parameters. */
2336-
currentQuery = psprintf("%s,$%d", currentQuery, currentColumn + 1);
2337-
nulls[currentColumn] = row->isNull[currentColumn];
2338+
currentQuery = psprintf("%s,$%d", currentQuery, paramIndex + 1);
2339+
nulls[paramIndex] = row->isNull[currentColumn];
2340+
paramIndex++;
23382341
currentColumn++;
23392342
}
23402343
row = row->nextRow;
@@ -2354,14 +2357,8 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
23542357

23552358
src = psprintf("Insert into %s values %s", finalTableName, query);
23562359
if ((rc = SPI_connect()) < 0)
2357-
{
2358-
/* Reset dialect. */
2359-
set_config_option("babelfishpg_tsql.sql_dialect", "tsql",
2360-
GUC_CONTEXT_CONFIG,
2361-
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
23622360
elog(ERROR, "SPI_connect() failed in TDS Listener "
23632361
"with return code %d", rc);
2364-
}
23652362

23662363
rc = SPI_execute_with_args(src,
23672364
nargs, argtypes,
@@ -2370,10 +2367,6 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
23702367

23712368
if (rc != SPI_OK_INSERT)
23722369
{
2373-
/* Reset dialect. */
2374-
set_config_option("babelfishpg_tsql.sql_dialect", "tsql",
2375-
GUC_CONTEXT_CONFIG,
2376-
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
23772370
elog(ERROR, "Failed to insert in the underlying table for table-valued parameter: %d", rc);
23782371
}
23792372

@@ -2384,10 +2377,15 @@ TdsRecvTypeTable(const char *message, const ParameterToken token)
23842377
if (!xactStarted)
23852378
CommitTransactionCommand();
23862379

2380+
}
2381+
}
2382+
PG_FINALLY();
2383+
{
23872384
set_config_option("babelfishpg_tsql.sql_dialect", "tsql",
23882385
GUC_CONTEXT_CONFIG,
23892386
PGC_S_SESSION, GUC_ACTION_SAVE, true, 0, false);
23902387
}
2388+
PG_END_TRY();
23912389

23922390
/* Free all the pointers. */
23932391
while (token->tvpInfo->rowData)

test/JDBC/expected/TestTvp.out

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,3 +51,16 @@ int
5151
drop procedure table_variable_vu_proc1;
5252
drop type table_variable_vu_type;
5353
drop type tableType;
54+
55+
-- Multi-row TVP test
56+
create type tvp_multirow_test_type as table (a int, b smallint, c bigint, d tinyint, e bit, f char(10), g nchar(10), h varchar(10), i nvarchar(10), l varbinary(10), m binary(10), n date, o datetime, p money, q uniqueidentifier, r float, s real, t numeric(4,3), u decimal(5,3), v time, w datetime2)
57+
prepst#!#Select * from ? order by a#!#tvp|-|tvp_multirow_test_type|-|utils/tvp-dotnet-multirow.csv|-|utils/tvp-dotnet-multirow.csv
58+
~~START~~
59+
int#!#smallint#!#bigint#!#tinyint#!#bit#!#char#!#nchar#!#varchar#!#nvarchar#!#varbinary#!#binary#!#date#!#datetime#!#money#!#uniqueidentifier#!#float#!#real#!#numeric#!#numeric#!#time#!#datetime2
60+
<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>#!#<NULL>
61+
1#!#1#!#100#!#10#!#1#!#hello #!#world #!#foo#!#bar#!#0A1B#!#0C0D0000000000000000#!#2023-01-15#!#2023-01-15 08:30:00.0#!#100.5000#!#CE8AF10A-2709-43B0-9E4E-A02753929D17#!#3.14#!#2.71#!#1.234#!#12.345#!#08:30:00.0000000#!#2023-01-15 08:30:00.0000000
62+
2#!#<NULL>#!#200#!#<NULL>#!#0#!#<NULL>#!#test #!#abc#!#<NULL>#!#<NULL>#!#1A2B0000000000000000#!#2024-06-20#!#<NULL>#!#<NULL>#!#A1B2C3D4-E5F6-7890-ABCD-EF1234567890#!#<NULL>#!#1.41#!#<NULL>#!#99.999#!#12:00:00.0000000#!#2024-06-20 12:00:00.0000000
63+
3#!#3#!#<NULL>#!#30#!#<NULL>#!#data #!#<NULL>#!#xyz#!#nval#!#FFEE#!#<NULL>#!#<NULL>#!#2025-12-31 23:59:59.0#!#999.9900#!#<NULL>#!#2.718#!#<NULL>#!#9.876#!#<NULL>#!#<NULL>#!#2025-12-31 23:59:59.0000000
64+
~~END~~
65+
66+
drop type tvp_multirow_test_type;

test/JDBC/input/datatypes/TestTvp.txt

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,3 +14,8 @@ declare @var1 table_variable_vu_type insert into @var1 values ('1', 2, 3, 4) exe
1414
drop procedure table_variable_vu_proc1;
1515
drop type table_variable_vu_type;
1616
drop type tableType;
17+
18+
-- Multi-row TVP test
19+
create type tvp_multirow_test_type as table (a int, b smallint, c bigint, d tinyint, e bit, f char(10), g nchar(10), h varchar(10), i nvarchar(10), l varbinary(10), m binary(10), n date, o datetime, p money, q uniqueidentifier, r float, s real, t numeric(4,3), u decimal(5,3), v time, w datetime2)
20+
prepst#!#Select * from @a order by a#!#tvp|-|tvp_multirow_test_type|-|utils/tvp-dotnet-multirow.csv|-|utils/tvp-dotnet-multirow.csv
21+
drop type tvp_multirow_test_type;

test/JDBC/src/main/java/com/sqlsamples/CompareResults.java

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,19 @@ static Object parse_data(String result, String datatype, Logger logger) {
290290
|| datatype.equalsIgnoreCase("varbinary")
291291
|| datatype.equalsIgnoreCase("timestamp")
292292
|| datatype.equalsIgnoreCase("udt")) {
293-
return result;
293+
/* convert hex strings to byte[] for binary/varbinary types */
294+
int len = result.length();
295+
if (len % 2 != 0)
296+
throw new IllegalArgumentException("Hex string has odd length: " + result);
297+
byte[] bytes = new byte[len / 2];
298+
for (int idx = 0; idx < len; idx += 2) {
299+
int hi = Character.digit(result.charAt(idx), 16);
300+
int lo = Character.digit(result.charAt(idx + 1), 16);
301+
if (hi < 0 || lo < 0)
302+
throw new IllegalArgumentException("Invalid hex character in: " + result);
303+
bytes[idx / 2] = (byte) ((hi << 4) + lo);
304+
}
305+
return bytes;
294306
} else if (datatype.equalsIgnoreCase("decimal")
295307
|| datatype.equalsIgnoreCase("money")
296308
|| datatype.equalsIgnoreCase("smallmoney")
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
a-int,b-smallint,c-bigint,d-tinyint,e-bit,f-char-10,g-nchar-10,h-varchar-10,i-nvarchar-10,l-varbinary-10,m-binary-10,n-date,o-datetime,p-money,q-uniqueidentifier,r-float,s-real,t-numeric-4-3,u-decimal-5-3,v-time,w-datetime2
2+
1,1,100,10,TRUE,hello,world,foo,bar,0A1B,0C0D,2023-01-15,2023-01-15 08:30:00,100.50,ce8af10a-2709-43b0-9e4e-a02753929d17,3.14,2.71,1.234,12.345,08:30:00,2023-01-15 08:30:00
3+
2,<NULL>,200,<NULL>,FALSE,<NULL>,test,abc,<NULL>,<NULL>,1A2B,2024-06-20,<NULL>,<NULL>,a1b2c3d4-e5f6-7890-abcd-ef1234567890,<NULL>,1.41,<NULL>,99.999,12:00:00,2024-06-20 12:00:00
4+
3,3,<NULL>,30,<NULL>,data,<NULL>,xyz,nval,FFEE,<NULL>,<NULL>,2025-12-31 23:59:59,999.99,<NULL>,2.718,<NULL>,9.876,<NULL>,<NULL>,2025-12-31 23:59:59
5+
<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>,<NULL>

0 commit comments

Comments
 (0)