Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

ODBC: Fix for data loading failure in Power BI Desktop #627

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
27 changes: 22 additions & 5 deletions sql-odbc/src/IntegrationTests/ITODBCCatalog/test_odbc_catalog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,12 @@ const std::vector< std::string > flights_data_type = {
"float", "keyword", "integer", "keyword", "keyword", "keyword", "date",
"keyword", "keyword", "boolean", "float", "keyword", "keyword", "keyword",
"keyword", "keyword", "keyword", "keyword"};
const std::vector< short > flights_sql_data_type = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use SQL type contstants here instead? (eg. SQLWVARCHAR)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added

SQL_WVARCHAR, SQL_WVARCHAR, SQL_BIT, SQL_REAL, SQL_REAL,
SQL_WVARCHAR, SQL_INTEGER, SQL_REAL, SQL_WVARCHAR, SQL_INTEGER,
SQL_WVARCHAR, SQL_WVARCHAR, SQL_WVARCHAR, SQL_TYPE_TIMESTAMP, SQL_WVARCHAR,
SQL_WVARCHAR, SQL_BIT, SQL_REAL, SQL_WVARCHAR, SQL_WVARCHAR,
SQL_WVARCHAR, SQL_WVARCHAR, SQL_WVARCHAR, SQL_WVARCHAR, SQL_WVARCHAR};
const std::string flights_catalog_odfe = "odfe-cluster";
const std::string flights_catalog_elas = "elasticsearch";
const std::string flights_table_name = "kibana_sample_data_flights";
Expand Down Expand Up @@ -355,15 +361,16 @@ TEST_F(TestSQLColumns, FlightsValidation) {
binds.push_back(bind_info(2, SQL_C_CHAR));
binds.push_back(bind_info(3, SQL_C_CHAR));
binds.push_back(bind_info(4, SQL_C_CHAR));
binds.push_back(bind_info(5, SQL_C_SSHORT));
binds.push_back(bind_info(5, SQL_C_SHORT));
binds.push_back(bind_info(6, SQL_C_CHAR));
binds.push_back(bind_info(7, SQL_C_SLONG));
binds.push_back(bind_info(8, SQL_C_SLONG));
binds.push_back(bind_info(9, SQL_C_SSHORT));
binds.push_back(bind_info(10, SQL_C_SSHORT));
binds.push_back(bind_info(11, SQL_C_SSHORT));
binds.push_back(bind_info(12, SQL_C_CHAR));
binds.push_back(bind_info(13, SQL_C_CHAR));
binds.push_back(bind_info(14, SQL_C_SSHORT));
binds.push_back(bind_info(14, SQL_C_SHORT));
binds.push_back(bind_info(15, SQL_C_SSHORT));
binds.push_back(bind_info(16, SQL_C_SLONG));
binds.push_back(bind_info(17, SQL_C_SLONG));
Expand All @@ -390,16 +397,26 @@ TEST_F(TestSQLColumns, FlightsValidation) {
case 4:
EXPECT_EQ(it.AsString(), flights_column_name[column_idx]);
break;
case 5:
EXPECT_EQ(
it.AsString(),
std::to_string(flights_sql_data_type[column_idx]));
break;
case 6:
EXPECT_EQ(it.AsString(), flights_data_type[column_idx]);
break;
case 9:
case 10:
EXPECT_EQ(it.AsString(), flights_decimal_digits);
break;
case 10:
case 11:
EXPECT_EQ(it.AsString(), flights_num_prec_radix);
break;
case 16:
case 14:
EXPECT_EQ(
it.AsString(),
std::to_string(flights_sql_data_type[column_idx]));
break;
case 17:
EXPECT_EQ(it.AsString(), std::to_string(column_idx + 1));
break;
default:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -936,6 +936,7 @@ TEST_F(TestSQLDescribeCol, SingleColumnMetadata) {
EXPECT_EQ(single_col, m_column_name);
EXPECT_EQ(single_col_name_length, m_column_name_length);
EXPECT_EQ(single_col_data_type, m_data_type);
// TODO #628 - Investigate why value differs & fix validation accordingly
EXPECT_EQ(single_col_column_size, m_column_size);
EXPECT_EQ(single_col_decimal_digit, m_decimal_digits);
EXPECT_EQ(single_col_nullable, m_nullable);
Expand Down
41 changes: 39 additions & 2 deletions sql-odbc/src/odfesqlodbc/es_info.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,22 @@ const std::unordered_map< int, std::vector< int > > sql_es_type_map = {
{ES_TYPE_KEYWORD, ES_TYPE_TEXT, ES_TYPE_NESTED, ES_TYPE_OBJECT}},
{SQL_TYPE_TIMESTAMP, {ES_TYPE_DATETIME}}};

const std::unordered_map< std::string, int > data_name_data_type_map = {
{ES_TYPE_NAME_BOOLEAN, SQL_BIT},
{ES_TYPE_NAME_BYTE, SQL_TINYINT},
{ES_TYPE_NAME_SHORT, SQL_SMALLINT},
{ES_TYPE_NAME_INTEGER, SQL_INTEGER},
{ES_TYPE_NAME_LONG, SQL_BIGINT},
{ES_TYPE_NAME_HALF_FLOAT, SQL_REAL},
{ES_TYPE_NAME_FLOAT, SQL_REAL},
{ES_TYPE_NAME_DOUBLE, SQL_DOUBLE},
{ES_TYPE_NAME_SCALED_FLOAT, SQL_DOUBLE},
{ES_TYPE_NAME_KEYWORD, SQL_WVARCHAR},
{ES_TYPE_NAME_TEXT, SQL_WVARCHAR},
{ES_TYPE_NAME_DATE, SQL_TYPE_TIMESTAMP},
{ES_TYPE_NAME_OBJECT, SQL_WVARCHAR},
{ES_TYPE_NAME_NESTED, SQL_WVARCHAR}};

// Boilerplate code for easy column bind handling
class BindTemplate {
public:
Expand Down Expand Up @@ -476,8 +492,29 @@ void SetTableTuples(QResultClass *res, const TableResultSet res_type,
// information for its Data Preview window.
std::string catalog("");
bind_tbl[TABLES_CATALOG_NAME]->UpdateData((void *)catalog.c_str(), 0);
for (size_t i = 0; i < binds.size(); i++)
binds[i]->AssignData(&tuple[i]);

// TODO #630 - Revisit logic of adding tuples for SQLTables & SQLColumns
for (size_t i = 0; i < binds.size(); i++) {
// Add tuples for SQLColumns
if (binds.size() > COLUMNS_SQL_DATA_TYPE) {

Choose a reason for hiding this comment

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

Should we just report the extra columns regardless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The same method is used while adding tuples for SQLTables as well. SQLTables has only 5 columns whereas SQLColumns has 18.
So this check ensures that the data type column is updated for SQLColumns only. If we remove this check then we get Memory Corruption since SQLTables will try to access column which doesn't exist.

Copy link
Contributor

Choose a reason for hiding this comment

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

Almost wondering if it would be worth it to split some of this logic up, now that we're handling special cases for SQLTables & SQLColumns

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Created issue #630 to revisit the logic of assigning data for SQLTables & SQLColumns

// Add data type for data loading issue in Power BI Desktop
auto data_type = data_name_data_type_map
.find(bind_tbl[COLUMNS_TYPE_NAME]->AsString())->second;
if (i == COLUMNS_DATA_TYPE) {
set_tuplefield_int2(&tuple[COLUMNS_DATA_TYPE],
static_cast< short >(data_type));
} else if (i == COLUMNS_SQL_DATA_TYPE) {
set_tuplefield_int2(&tuple[COLUMNS_SQL_DATA_TYPE],
static_cast< short >(data_type));
} else {
binds[i]->AssignData(&tuple[i]);
}
}
// Add tuples for SQLTables
else {
binds[i]->AssignData(&tuple[i]);
}
}
};

// General case
Expand Down
30 changes: 15 additions & 15 deletions sql-odbc/src/odfesqlodbc/es_parse_result.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,21 @@ static const std::string JSON_KW_CURSOR = "cursor";

// clang-format on
const std::unordered_map< std::string, OID > type_to_oid_map = {
{"boolean", ES_TYPE_BOOL},
{"byte", ES_TYPE_INT2},
{"short", ES_TYPE_INT2},
{"integer", ES_TYPE_INT4},
{"long", ES_TYPE_INT8},
{"half_float", ES_TYPE_FLOAT4},
{"float", ES_TYPE_FLOAT4},
{"double", ES_TYPE_FLOAT8},
{"scaled_float", ES_TYPE_FLOAT8},
{"keyword", ES_TYPE_VARCHAR},
{"text", ES_TYPE_VARCHAR},
{"date", ES_TYPE_TIMESTAMP},
{"object", ES_TYPE_VARCHAR},
{"nested", ES_TYPE_VARCHAR},
{"date", ES_TYPE_DATE}};
{ES_TYPE_NAME_BOOLEAN, ES_TYPE_BOOL},
{ES_TYPE_NAME_BYTE, ES_TYPE_INT2},
{ES_TYPE_NAME_SHORT, ES_TYPE_INT2},
{ES_TYPE_NAME_INTEGER, ES_TYPE_INT4},
{ES_TYPE_NAME_LONG, ES_TYPE_INT8},
{ES_TYPE_NAME_HALF_FLOAT, ES_TYPE_FLOAT4},
{ES_TYPE_NAME_FLOAT, ES_TYPE_FLOAT4},
{ES_TYPE_NAME_DOUBLE, ES_TYPE_FLOAT8},
{ES_TYPE_NAME_SCALED_FLOAT, ES_TYPE_FLOAT8},
{ES_TYPE_NAME_KEYWORD, ES_TYPE_VARCHAR},
{ES_TYPE_NAME_TEXT, ES_TYPE_VARCHAR},
{ES_TYPE_NAME_DATE, ES_TYPE_TIMESTAMP},
{ES_TYPE_NAME_OBJECT, ES_TYPE_VARCHAR},
{ES_TYPE_NAME_VARCHAR, ES_TYPE_VARCHAR},
{ES_TYPE_NAME_DATE, ES_TYPE_DATE}};

#define ES_VARCHAR_SIZE (-2)
const std::unordered_map< OID, int16_t > oid_to_size_map = {
Expand Down
32 changes: 16 additions & 16 deletions sql-odbc/src/odfesqlodbc/es_types.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,22 +30,22 @@ extern "C" {
#define ES_TYPE_LO ? ? ? ? /* waiting for permanent type */
#endif

#define ES_TYPE_NAME_BOOLEAN "boolean";
#define ES_TYPE_NAME_BYTE "byte";
#define ES_TYPE_NAME_SHORT "short";
#define ES_TYPE_NAME_INTEGER "integer";
#define ES_TYPE_NAME_LONG "long";
#define ES_TYPE_NAME_HALF_FLOAT "half_float";
#define ES_TYPE_NAME_FLOAT "float";
#define ES_TYPE_NAME_DOUBLE "double";
#define ES_TYPE_NAME_SCALED_FLOAT "scaled_float";
#define ES_TYPE_NAME_KEYWORD "keyword";
#define ES_TYPE_NAME_TEXT "text";
#define ES_TYPE_NAME_NESTED "nested";
#define ES_TYPE_NAME_DATE "date";
#define ES_TYPE_NAME_OBJECT "object";
#define ES_TYPE_NAME_VARCHAR "varchar";
#define ES_TYPE_NAME_UNSUPPORTED "unsupported";
#define ES_TYPE_NAME_BOOLEAN "boolean"
#define ES_TYPE_NAME_BYTE "byte"
#define ES_TYPE_NAME_SHORT "short"
#define ES_TYPE_NAME_INTEGER "integer"
#define ES_TYPE_NAME_LONG "long"
#define ES_TYPE_NAME_HALF_FLOAT "half_float"
#define ES_TYPE_NAME_FLOAT "float"
#define ES_TYPE_NAME_DOUBLE "double"
#define ES_TYPE_NAME_SCALED_FLOAT "scaled_float"
#define ES_TYPE_NAME_KEYWORD "keyword"
#define ES_TYPE_NAME_TEXT "text"
#define ES_TYPE_NAME_NESTED "nested"
#define ES_TYPE_NAME_DATE "date"
#define ES_TYPE_NAME_OBJECT "object"
#define ES_TYPE_NAME_VARCHAR "varchar"
#define ES_TYPE_NAME_UNSUPPORTED "unsupported"

#define MS_ACCESS_SERIAL "int identity"
#define ES_TYPE_BOOL 16
Expand Down