Data conversions: numeric types #9
Conversation
build.bat suite=my_test.cc
The refactoring avoid calculating the lenght of the writen wide char string, since that's nearly always known before the call.
This only needs to happen when all indicator buffers are NULL. -> use REC_IS_BOUND for this. Also fixed a typo that was checking twice for ARD record type (insead of ARD or APD).
this allows for faster log lines related to one test
Converting between ES/SQL and SQL C types will now work.
- also, pipe all conversions of boolean through longlogn conversion, except the strings ones.
- add unit testing for them - small refactoring of ints conversions to reduce code duplication
|
Thanks, fixed them. |
driver/handles.c
Outdated
| (SQLLEN)(intptr_t)ValuePtr); | ||
| /* rec field's type is signed :/; a negative is dangerous l8r */ | ||
| if ((SQLLEN)(intptr_t)ValuePtr < 0) { | ||
| ERRH(desc, "octet length attribute can't be negative (%lu)", |
There was a problem hiding this comment.
SQLLEN is signed, so should %lu be changed to %ld? And, as above, it might avoid future portability problems to case to long rather than SQLLEN on the line below.
There was a problem hiding this comment.
Fixed, thanks.
I'm trying to use the SQL C types and put asserts wherever this would be critical.
The SQLLEN is defined as an INT64 (for _WIN64), so a long long.
So yes, the logging descriptors for ints would need some extending/rework for 32bits, indeed (but I'd like to have a look into it sooner).
driver/util.c
Outdated
| SQLRETURN write_wstr(SQLHANDLE hnd, SQLWCHAR *dest, wstr_st *src, | ||
| SQLSMALLINT /*B*/avail, SQLSMALLINT /*B*/*usedp) | ||
| { | ||
| size_t awail; |
There was a problem hiding this comment.
I think it's a bit of a trap to have variables avail and awail in the same method. Maybe rename awail to wideAvail?
There was a problem hiding this comment.
True. Fixed, thanks.
driver/util.c
Outdated
|
|
||
| INFOH(hnd, "not enough buffer size to write required string (plus " | ||
| "terminator): `" LWPD "` [%zd]; available: %d.", | ||
| LWSTR(src), src->cnt, awail); |
There was a problem hiding this comment.
awail is size_t, so I think the last format should be %zu?
| std::wstring wstr(nameLen, L' '); | ||
| ASSERT_TRUE(mbstowcs(&wstr[0], testName, nameLen + 1) != (size_t)-1); | ||
| ret = ATTACH_SQL(stmt, &wstr[0], nameLen); | ||
| ASSERT_TRUE(SQL_SUCCEEDED(ret)); |
There was a problem hiding this comment.
It looks like the test files are indented with spaces. Would it be better to use either tabs or spaces consistently through the whole repo?
There was a problem hiding this comment.
It might, yes. GoogleTest framework uses spaces for indenting so I've just continued that, but yes, it would be more logical to be consistent within the repo. I'll apply this change at a later point.
build.bat
Outdated
| @@ -1,4 +1,4 @@ | |||
| @echo off | |||
| echo off | |||
There was a problem hiding this comment.
Does removing the @ mean that echo off itself now gets printed to the console? (It would have done 25 years ago but maybe not now.) If so, was removing it intentional?
There was a problem hiding this comment.
Yes, indeed, @ (still) mutes the statement itself (in my cmd shell).
But the removal wasn't quite intentional, I've added it back, thanks!
This PR continues the data conversion development item with conversions for the the integers and float point types.
The JSON library that reads ES' replies to queries transforms all the integers received into either
long longordoubletypes. The driver handles the conversion from these types to the type the application would like them converted to (within the allowed possibilities): strings (ansi/unicode), integers, floats, bit, binary and numeric.