Skip to content

Commit

Permalink
Address code review
Browse files Browse the repository at this point in the history
  • Loading branch information
Jerry Leung committed Mar 19, 2021
1 parent 0b0fff2 commit 75a80aa
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/IntegrationTests/ITODBCHelper/it_odbc_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#define SQLSTATE_RESTRICTED_DATA_TYPE_ERROR (SQLWCHAR*)L"07006"
#define SQLSTATE_INVALID_DESCRIPTOR_INDEX (SQLWCHAR*)L"07009"
#define SQLSTATE_GENERAL_ERROR (SQLWCHAR*)L"HY000"
#define SQLSTATE_INVALID_STRING_OR_BUFFER_LENGTH (SQLWCHAR*)L"HY090"
#define SQLSTATE_INVALID_DESCRIPTOR_FIELD_IDENTIFIER (SQLWCHAR*)L"HY091"
#define SQLSTATE_NUMERIC_VALUE_OUT_OF_RANGE (SQLWCHAR*) L"HY019"
#define SQLSTATE_STRING_CONVERSION_ERROR (SQLWCHAR*)L"22018"
Expand Down
56 changes: 56 additions & 0 deletions src/IntegrationTests/ITODBCResults/test_odbc_results.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4315,6 +4315,7 @@ TEST_F(TestSQLDescribeCol, INTEGER_COLUMN) {
EXPECT_EQ((SQLULEN)10, column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, DOUBLE_COLUMN) {
Expand All @@ -4336,6 +4337,7 @@ TEST_F(TestSQLDescribeCol, DOUBLE_COLUMN) {
EXPECT_EQ((SQLULEN)15, column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, BIGINT_COLUMN) {
Expand All @@ -4357,6 +4359,7 @@ TEST_F(TestSQLDescribeCol, BIGINT_COLUMN) {
EXPECT_EQ((SQLULEN)19, column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, BOOLEAN_COLUMN) {
Expand All @@ -4378,6 +4381,7 @@ TEST_F(TestSQLDescribeCol, BOOLEAN_COLUMN) {
EXPECT_EQ((SQLULEN)1, column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, VARCHAR_COLUMN) {
Expand All @@ -4400,6 +4404,7 @@ TEST_F(TestSQLDescribeCol, VARCHAR_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, TIMESERIES_COLUMN) {
Expand Down Expand Up @@ -4433,6 +4438,7 @@ TEST_F(TestSQLDescribeCol, TIMESERIES_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, ARRAY_COLUMN) {
Expand All @@ -4457,6 +4463,7 @@ TEST_F(TestSQLDescribeCol, ARRAY_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, ROW_COLUMN) {
Expand All @@ -4479,6 +4486,7 @@ TEST_F(TestSQLDescribeCol, ROW_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, NULL_COLUMN) {
Expand All @@ -4501,6 +4509,7 @@ TEST_F(TestSQLDescribeCol, NULL_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, TIMESTAMP_COLUMN) {
Expand All @@ -4523,6 +4532,7 @@ TEST_F(TestSQLDescribeCol, TIMESTAMP_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, DATE_COLUMN) {
Expand All @@ -4545,6 +4555,7 @@ TEST_F(TestSQLDescribeCol, DATE_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, TIME_COLUMN) {
Expand All @@ -4567,6 +4578,7 @@ TEST_F(TestSQLDescribeCol, TIME_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, INTERVAL_YEAR_TO_MONTH_COLUMN) {
Expand All @@ -4589,6 +4601,7 @@ TEST_F(TestSQLDescribeCol, INTERVAL_YEAR_TO_MONTH_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, INTERVAL_DAY_TO_SECOND_COLUMN) {
Expand All @@ -4611,6 +4624,7 @@ TEST_F(TestSQLDescribeCol, INTERVAL_DAY_TO_SECOND_COLUMN) {
EXPECT_EQ((SQLULEN)expected.size(), column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, OUT_OF_INDEX_COLUMN) {
Expand All @@ -4627,6 +4641,47 @@ TEST_F(TestSQLDescribeCol, OUT_OF_INDEX_COLUMN) {
&data_type, &column_size, &decimal_digits, &nullable);
EXPECT_EQ(SQL_ERROR, ret);
EXPECT_TRUE(CheckSQLSTATE(SQL_HANDLE_STMT, m_hstmt, SQLSTATE_INVALID_DESCRIPTOR_INDEX));
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, TRUNCATED_COLUMN_NAME_COLUMN) {
SQLRETURN ret = SQL_ERROR;
std::wstring columns = L"INTEGER\'1\'";
QueryFetch(columns, table_name, single_row, &m_hstmt);
SQLTCHAR column_name[1];
SQLSMALLINT column_name_length;
SQLSMALLINT data_type;
SQLULEN column_size;
SQLSMALLINT decimal_digits;
SQLSMALLINT nullable;
ret = SQLDescribeCol(m_hstmt, 1, column_name, 1, &column_name_length,
&data_type, &column_size, &decimal_digits, &nullable);
EXPECT_EQ(SQL_SUCCESS_WITH_INFO, ret);
EXPECT_TRUE(CheckSQLSTATE(SQL_HANDLE_STMT, m_hstmt,
SQLSTATE_STRING_DATA_RIGHT_TRUNCATED));
std::wstring truncated_column_name = L"_";
EXPECT_STREQ(truncated_column_name.c_str(), column_name);
std::wstring expected_column_name = L"_col0";
EXPECT_EQ((SQLSMALLINT)expected_column_name.size(), column_name_length);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, INVALID_STRING_OR_BUFFER_LENGTH) {
SQLRETURN ret = SQL_ERROR;
std::wstring columns = L"INTEGER\'1\'";
QueryFetch(columns, table_name, single_row, &m_hstmt);
SQLTCHAR column_name[60];
SQLSMALLINT column_name_length;
SQLSMALLINT data_type;
SQLULEN column_size;
SQLSMALLINT decimal_digits;
SQLSMALLINT nullable;
ret = SQLDescribeCol(m_hstmt, 1, column_name, -1, &column_name_length,
&data_type, &column_size, &decimal_digits, &nullable);
EXPECT_EQ(SQL_ERROR, ret);
EXPECT_TRUE(CheckSQLSTATE(SQL_HANDLE_STMT, m_hstmt,
SQLSTATE_INVALID_STRING_OR_BUFFER_LENGTH));
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLDescribeCol, MULTIPLE_COLUMNS) {
Expand Down Expand Up @@ -4675,6 +4730,7 @@ TEST_F(TestSQLDescribeCol, MULTIPLE_COLUMNS) {
EXPECT_EQ((SQLULEN)1, column_size);
EXPECT_EQ(0, decimal_digits);
EXPECT_EQ(SQL_NULLABLE_UNKNOWN, nullable);
LogAnyDiagnostics(SQL_HANDLE_STMT, m_hstmt, ret);
}

TEST_F(TestSQLMoreResults, NoData_noquery) {
Expand Down
6 changes: 6 additions & 0 deletions src/odfesqlodbc/odbcapi.c
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,12 @@ RETCODE SQL_API SQLDescribeCol(HSTMT StatementHandle, SQLUSMALLINT ColumnNumber,
if (SC_connection_lost_check(stmt, __FUNCTION__))
return SQL_ERROR;

if (BufferLength < 0) {
SC_set_error(stmt, STMT_INVALID_STRING_OR_BUFFER_LENGTH_ERROR,
"Invalid string or buffer length", __FUNCTION__);
return SQL_ERROR;
}

ENTER_STMT_CS(stmt);
SC_clear_error(stmt);
ret = API_DescribeCol(StatementHandle, ColumnNumber, ColumnName,
Expand Down
6 changes: 6 additions & 0 deletions src/odfesqlodbc/odbcapiw.c
Original file line number Diff line number Diff line change
Expand Up @@ -238,6 +238,12 @@ RETCODE SQL_API SQLDescribeColW(HSTMT StatementHandle,
if (SC_connection_lost_check(stmt, __FUNCTION__))
return SQL_ERROR;

if (BufferLength < 0) {
SC_set_error(stmt, STMT_INVALID_STRING_OR_BUFFER_LENGTH_ERROR,
"Invalid string or buffer length", __FUNCTION__);
return SQL_ERROR;
}

buflen = 0;
if (BufferLength > 0)
buflen = BufferLength * 3;
Expand Down
3 changes: 2 additions & 1 deletion src/odfesqlodbc/statement.c
Original file line number Diff line number Diff line change
Expand Up @@ -788,7 +788,8 @@ static const struct {
{STMT_INVALID_NULL_ARG, "HY009", "S1009"},
{STMT_NO_RESPONSE, "08S01", "08S01"},
{STMT_COMMUNICATION_ERROR, "08S01", "08S01"},
{STMT_STRING_CONVERSION_ERROR, "22018", "22005"}};
{STMT_STRING_CONVERSION_ERROR, "22018", "22005"},
{STMT_INVALID_STRING_OR_BUFFER_LENGTH_ERROR, "HY090", "HY090"}};

static ES_ErrorInfo *SC_create_errorinfo(const StatementClass *self,
ES_ErrorInfo *eserror_fail_safe) {
Expand Down
3 changes: 2 additions & 1 deletion src/odfesqlodbc/statement.h
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,8 @@ enum {
STMT_INVALID_NULL_ARG,
STMT_NO_RESPONSE,
STMT_COMMUNICATION_ERROR,
STMT_STRING_CONVERSION_ERROR
STMT_STRING_CONVERSION_ERROR,
STMT_INVALID_STRING_OR_BUFFER_LENGTH_ERROR
};

/* statement types */
Expand Down

0 comments on commit 75a80aa

Please sign in to comment.