Skip to content

Fix varchar limitation when max-len-attr is set #231

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
Mar 27, 2020
Merged

Fix varchar limitation when max-len-attr is set #231

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4d1d99c
Fix varchar limitation when max-len-attr is set
bpintea Mar 27, 2020
05baebf
address review comments
bpintea Mar 27, 2020
6e25d7f
Merge branch 'master' into fix/varchar_lim_with_max_length
bpintea Mar 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 15 additions & 7 deletions driver/catalogue.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ SQLSMALLINT fetch_server_attr(esodbc_dbc_st *dbc, SQLINTEGER attr_id,
} else {
if (1 < row_cnt) {
WARNH(dbc, "more than one value (%lld) available for "
"attribute %ld; picking first.", row_cnt, attr_id);
"attribute %ld; picking first.", (int64_t)row_cnt, attr_id);
}

if (! SQL_SUCCEEDED(EsSQLBindCol(stmt, /*col#*/1, SQL_C_WCHAR, buff,
Expand Down Expand Up @@ -577,17 +577,17 @@ SQLRETURN TEST_API update_varchar_defs(esodbc_stmt_st *stmt)
"{\"name\":\"TABLE_SCHEM\", \"type\":\"keyword\"},"
"{\"name\":\"TABLE_NAME\", \"type\":\"keyword\"},"
"{\"name\":\"COLUMN_NAME\", \"type\":\"keyword\"},"
"{\"name\":\"DATA_TYPE\", \"type\":\"integer\"},"
"{\"name\":\"DATA_TYPE\", \"type\":\"short\"},"
"{\"name\":\"TYPE_NAME\", \"type\":\"keyword\"},"
"{\"name\":\"COLUMN_SIZE\", \"type\":\"integer\"},"
"{\"name\":\"BUFFER_LENGTH\", \"type\":\"integer\"},"
"{\"name\":\"DECIMAL_DIGITS\", \"type\":\"integer\"},"
"{\"name\":\"NUM_PREC_RADIX\", \"type\":\"integer\"},"
"{\"name\":\"NULLABLE\", \"type\":\"integer\"},"
"{\"name\":\"DECIMAL_DIGITS\", \"type\":\"short\"},"
"{\"name\":\"NUM_PREC_RADIX\", \"type\":\"short\"},"
"{\"name\":\"NULLABLE\", \"type\":\"short\"},"
"{\"name\":\"REMARKS\", \"type\":\"keyword\"},"
"{\"name\":\"COLUMN_DEF\", \"type\":\"keyword\"},"
"{\"name\":\"SQL_DATA_TYPE\", \"type\":\"integer\"},"
"{\"name\":\"SQL_DATETIME_SUB\", \"type\":\"integer\"},"
"{\"name\":\"SQL_DATA_TYPE\", \"type\":\"short\"},"
"{\"name\":\"SQL_DATETIME_SUB\", \"type\":\"short\"},"
"{\"name\":\"CHAR_OCTET_LENGTH\", \"type\":\"integer\"},"
"{\"name\":\"ORDINAL_POSITION\", \"type\":\"integer\"},"
"{\"name\":\"IS_NULLABLE\", \"type\":\"keyword\"}"
Expand All @@ -605,9 +605,15 @@ SQLRETURN TEST_API update_varchar_defs(esodbc_stmt_st *stmt)
long row_cnt;
wstr_st dest = {0};
size_t pos;
SQLULEN max_length;
cstr_st u8mb;
wstr_st *lim = &HDRH(stmt)->dbc->varchar_limit_str;

/* save and reset SQL_ATTR_MAX_LENGTH attribute, it'll interfere with
* reading lenght of avail data with SQLGetData() otherwise. */
Copy link

@droberts195 droberts195 Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo: lenght -> length

Copy link
Collaborator Author

@bpintea bpintea Mar 27, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to install a precommit check for this one. Thanks, fixed them all.

max_length = stmt->max_length;
stmt->max_length = 0;

/* check that we have as many columns as members in target row struct */
ret = EsSQLNumResultCols(stmt, &col_cnt);
if (! SQL_SUCCEEDED(ret)) {
Expand Down Expand Up @@ -689,6 +695,8 @@ SQLRETURN TEST_API update_varchar_defs(esodbc_stmt_st *stmt)
free(dest.str);
dest.cnt = 0;
}
/* reinstate any saved SQL_ATTR_MAX_LENGTH value */
stmt->max_length = max_length;
return ret;
}

Expand Down
25 changes: 25 additions & 0 deletions test/test_catalogue.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,11 @@ TEST_F(Catalogue, Columns_update_varchar_defs) {
"]"
"}";

/* setting this attribute should not affect the outcome */
ret = SQLSetStmtAttr(stmt, SQL_ATTR_MAX_LENGTH, (SQLPOINTER)INT_MAX,
SQL_IS_UINTEGER);
ASSERT_TRUE(SQL_SUCCEEDED(ret));

prepareStatement(response);

/* needs to be lower than ESODBC_MAX_KEYWORD_PRECISION (~32k) */
Expand Down Expand Up @@ -120,6 +125,26 @@ TEST_F(Catalogue, Columns_update_varchar_defs) {
ASSERT_EQ(val, VARCHAR_LIMIT);
}

/* above SYS COLUMNS result contains wrong (integer) type for the following
* columns that should be short */
const SQLUSMALLINT short_cols[] = {
SQLCOLS_IDX_DATA_TYPE,
SQLCOLS_IDX_DECIMAL_DIGITS,
SQLCOLS_IDX_NUM_PREC_RADIX,
SQLCOLS_IDX_NULLABLE,
SQLCOLS_IDX_SQL_DATA_TYPE,
SQLCOLS_IDX_SQL_DATETIME_SUB
};
SQLWCHAR col_name[128];
SQLSMALLINT col_name_len, sql_type, scale, nullable;
SQLULEN col_size;
for (size_t i = 0; i < sizeof(short_cols)/sizeof(short_cols[0]); i ++) {
ret = SQLDescribeCol(stmt, short_cols[i], col_name, sizeof(col_name),
&col_name_len, &sql_type, &col_size, &scale, &nullable);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ASSERT_EQ(sql_type, SQL_SMALLINT);
}

HDRH(stmt)->dbc->varchar_limit = 0;
memset(&HDRH(stmt)->dbc->varchar_limit_str, 0, sizeof(wstr_st));

Expand Down
61 changes: 61 additions & 0 deletions test/test_queries.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,67 @@ TEST_F(Queries, SQLNumParams_duplicates_escape) {
ASSERT_EQ(params, 2);
}

TEST_F(Queries, SQLDescribeCol_varchar_lim) {

# undef COL_NAME
# define COL_NAME "SQLDescribeCol_varchar_lim"
# define VARCHAR_LIMIT 333
# define CONN_STR CONNECT_STRING "VarcharLimit=" STR(VARCHAR_LIMIT) ";"

const char json_answer[] = "\
{\
\"columns\": [\
{\"name\": \"" COL_NAME "\", \"type\": \"text\"},\
{\"name\": \"binary\", \"type\": \"binary\"}\
],\
\"rows\": [\
[\"foo\", \"binary\"]\
]\
}\
";

/* set varchar limit: this is set onto the varchar types structures, so the
* DBC needs "reconnecting" and the corresponding DSN param set */
ret = SQLFreeHandle(SQL_HANDLE_STMT, stmt);
assert(SQL_SUCCEEDED(ret));
ret = SQLFreeHandle(SQL_HANDLE_DBC, dbc);
assert(SQL_SUCCEEDED(ret));
ret = SQLAllocHandle(SQL_HANDLE_DBC, env, &dbc);
assert(SQL_SUCCEEDED(ret));
cstr_st types = {0};
types.str = (SQLCHAR *)strdup(SYSTYPES_ANSWER);
assert(types.str != NULL);
types.cnt = sizeof(SYSTYPES_ANSWER) - 1;
ret = SQLDriverConnect(dbc, (SQLHWND)&types, (SQLWCHAR *)CONN_STR,
sizeof(CONN_STR) / sizeof(CONN_STR[0]) - 1, NULL, 0, NULL,
ESODBC_SQL_DRIVER_TEST);
assert(SQL_SUCCEEDED(ret));
ret = SQLAllocHandle(SQL_HANDLE_STMT, dbc, &stmt);
assert(SQL_SUCCEEDED(ret));
assert(stmt != NULL);

prepareStatement(json_answer);

SQLWCHAR col_name[sizeof(COL_NAME)];
SQLSMALLINT col_name_len, sql_type, scale, nullable;
SQLULEN col_size;
/* text column */
ret = SQLDescribeCol(stmt, /*col#*/1, col_name, sizeof(col_name),
&col_name_len, &sql_type, &col_size, &scale, &nullable);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ASSERT_EQ(col_name_len, sizeof(COL_NAME) - 1);
ASSERT_STREQ(col_name, MK_WPTR(COL_NAME));
ASSERT_EQ(sql_type, ES_WVARCHAR_SQL);
ASSERT_EQ(col_size, VARCHAR_LIMIT); /* limit enforced */
ASSERT_EQ(nullable, SQL_NULLABLE_UNKNOWN);

/* binary column */
ret = SQLDescribeCol(stmt, /*col#*/2, col_name, sizeof(col_name),
&col_name_len, &sql_type, &col_size, &scale, &nullable);
ASSERT_TRUE(SQL_SUCCEEDED(ret));
ASSERT_EQ(col_size, INT_MAX); /* binary col's length must not be affected */
}

} // test namespace

/* vim: set noet fenc=utf-8 ff=dos sts=0 sw=4 ts=4 : */