Driver/data_source features detection completion #18
Conversation
since dependenices are now (local) git subtrees.
workaround LibreOffice's way of providing the TableType parameter: remove trailing commas, if any
- broke SQLGetInfo() into functions covering delimited topics; - added answers to all requested attributes (with two small exceptions); - removed defines from defs.h for those parameters that aren't used anywhere else but in one place and that aren't going to change with ES/SQL further development.
driver/connect.c
Outdated
| @@ -473,7 +473,7 @@ static SQLRETURN process_config(esodbc_dbc_st *dbc, esodbc_dsn_attrs_st *attrs) | |||
| } | |||
| n = WCS2U8(urlw, cnt, dbc->url, n); | |||
| if (! n) { | |||
| ERRNH(dbc, "failed to U8 convert URL `" LWPDL "` [%d].",cnt, urlw, cnt); | |||
| ERRNH(dbc, "failed to U8 convert URL `"LWPDL"` [%d].",cnt, urlw, cnt); | |||
There was a problem hiding this comment.
In other places there are spaces between LWPDL and the surrounding double quotes.
There was a problem hiding this comment.
Changed it back to stay consistent, thanks.
driver/util.c
Outdated
| @@ -448,7 +448,7 @@ size_t json_escape_overlapping(char *str, size_t inlen, size_t outlen) | |||
| * EsSQLColAttributeW). */ | |||
| /* | |||
| * Copy a WSTR back to application; typically with non-SQLFetch() calls. | |||
| * The WSTR must not count the 0-tem. | |||
| * The WSTR must not count the 0-tem, but must include it. | |||
driver/connect.c
Outdated
| (SQLUINTEGER)(uintptr_t)Value); | ||
| dbc->txn_isolation = (SQLUINTEGER)(uintptr_t)Value; | ||
| ERRH(dbc, "no support for transactions available."); | ||
| //RET_HDIAGS(dbc, SQL_STATE_HYC00); |
There was a problem hiding this comment.
It might be worth adding a comment here to say it's deliberate to tolerate requests to change the transaction isolation level (I guess because certain clients require it?), but that the commented out line would be the way to return an error if we ever decided to do that.
There was a problem hiding this comment.
I've added a comment to indicate that purpose of leaving there commented error returning line. (The app should not set the transaction level, since the source is read-only. If any app will rely on setting it, though, the driver should return an error right away, rather than ignore the action of setting it.)
driver/handles.c
Outdated
| case SQL_ATTR_CONCURRENCY: | ||
| DBGH(stmt, "setting concurrency: %llu.", (SQLULEN)ValuePtr); | ||
| if ((SQLULEN)ValuePtr != SQL_CONCUR_READ_ONLY) { | ||
| WARNH(stmt, "concurrency force-changed to read-only (%llu).", |
There was a problem hiding this comment.
Given the definition of SQL state 01S02 do you think it might be clearer to phrase the warning as "requested concurrency substituted with read-only (%llu)."?
There was a problem hiding this comment.
I've rephrased the error message to the indicated one, thanks.
And same for the other three locations.
driver/handles.c
Outdated
| case SQL_ATTR_MAX_ROWS: | ||
| DBGH(stmt, "setting max rows: %llu.", (SQLULEN)ValuePtr); | ||
| if ((SQLULEN)ValuePtr != 0) { | ||
| WARNH(stmt, "max rows force-changed to 0."); |
There was a problem hiding this comment.
Similar to above maybe "requested max rows substituted with 0."?
This PR brings completion to the list of attributes used in the detection of feature available in the driver and data source. The detection is based on a list of ~170 attributes around:
The PR offers an answer on all of them (safe two exceptions). Currently, these answers are static (locally generated), a future direction might be to have some of them (DS caps, scalars and conversions) read from the server, as the single source of truth.