Skip to content
This repository has been archived by the owner on Jun 7, 2021. It is now read-only.

[TRAFODION-306] Re-fix #584

Merged
merged 1 commit into from Jul 7, 2016
Merged

[TRAFODION-306] Re-fix #584

merged 1 commit into from Jul 7, 2016

Conversation

ryzuo
Copy link
Contributor

@ryzuo ryzuo commented Jul 7, 2016

Re-fix SQLGetTypeInfo for JDBC broken.

JDBC and ODBC has different standards of Data Type Code definitions, and we share the same query for getting the information, last commit yesterday broke JDBC getTypeInfo() API. Re-fix it for JDBC API functionality.

@ryzuo ryzuo changed the title [Fix JIRA306] [TRAFODION-306] Re-fix Jul 7, 2016
@Traf-Jenkins
Copy link

Check Test Started: https://jenkins.esgyn.com/job/Check-PR-master/884/

@Traf-Jenkins
Copy link

}
else
{
SqlTypeCode_Date = SQL_DATE;
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't Trafodion ODBC conforms to ODBC 3.0? If so, is it needed to use SQL_TYPE_DATE, SQL_TYPE_TIME and SQL_TYPE_TIMESTAMP which is 91,92 and 93 respectively. sql.h has the following

/* One-parameter shortcuts for date/time data types */
#if (ODBCVER >= 0x0300)
#define SQL_TYPE_DATE 91
#define SQL_TYPE_TIME 92
#define SQL_TYPE_TIMESTAMP 93
#endif

While the SQL_DATE is sqlext.h

/* SQL extended datatypes /
#define SQL_DATE 9
#if (ODBCVER >= 0x0300)
#define SQL_INTERVAL 10
#endif /
ODBCVER >= 0x0300 /
#define SQL_TIME 10
#define SQL_TIMESTAMP 11
#define SQL_LONGVARCHAR (-1)
#define SQL_BINARY (-2)
#define SQL_VARBINARY (-3)
#define SQL_LONGVARBINARY (-4)
#define SQL_BIGINT (-5)
#define SQL_TINYINT (-6)
#define SQL_BIT (-7)
#if (ODBCVER >= 0x0350)
#define SQL_GUID (-11)
#endif /
ODBCVER >= 0x0350 */

What is expected by the ODBC application for the date, time and timestamp from the ODBC driver conforming to ODBC 3.0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@selvaganesang You are right about this. But our driver driver will convert old definition to version 3 value. Basically it is compatible with ODBC 3.0 and 2.1, but relying on how the environment attribute SQL_ATTR_ODBC_VERSION is set by application. This problem was detected by Coast test cases, and Coast expects old value. So I did not realize if there would be any impact by returning the old value. We don't have plenty test cases now to test all considerable conditions, however the coast just looks fine now.
I don't know is there any application is running on our ODBC, which relies on this catalog API now, and it might be running into this compatibility problem. If the SQL_ATTR_ODBC_VERSION is not set properly, it might. If there's any app has the problem, I'd like to know then I will see what I can do about it.
5 pic
6 pic

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we are confusing the compile time defines ODBCVER with run time ODBC application version SQL_ATTR_ODBC_VERSION. By default sql.h sets the ODBCVER to 0x351. The trafodion code is not redefining it. Hence, Trafodion ODBC driver conforms to 3.5 version and ODBC driver should be returning SQL_TYPE_DATE, SQL_TYPE_TIME and SQL_TYPE_TIMESTAMP. It is the responsibility of the driver manager to do the needed mapping to ODBC 2.0 and ODBC 3.0 applications. See " Datetime Data Type Changes" in ODBC 3.x specification

I have copied the portion of it here
In ODBC 3.x, the identifiers for date, time, and timestamp SQL data types have changed from SQL_DATE,
SQL_TIME, and SQL_TIMESTAMP (with instances of #define in the header file of 9, 10, and 11) to SQL_TYPE_DATE,
SQL_TYPE_TIME, and SQL_TYPE_TIMESTAMP (with instances of #define in the header file of 91, 92, and 93),
respectively. The corresponding C type identifiers have changed from SQL_C_DATE, SQL_C_TIME, and
SQL_C_TIMESTAMP to SQL_C_TYPE_DATE, SQL_C_TYPE_TIME, and SQL_C_TYPE_TIMESTAMP, respectively.
The column size and decimal digits returned for the SQL datetime data types in ODBC 3.x are the same as the
precision and scale returned for them in ODBC 2.x. These values are different than the values in the
SQL_DESC_PRECISION and SQL_DESC_SCALE descriptor fields. (For more information, see Column Size, Decimal
Digits, Transfer Octet Length, and Display Size.)
These changes affect SQLDescribeCol, SQLDescribeParam, and SQLColAttribute; SQLBindCol,
SQLBindParameter, and SQLGetData; and SQLColumns, SQLGetTypeInfo, SQLProcedureColumns,
SQLStatistics, and SQLSpecialColumns.
The following table shows how the ODBC 3.x Driver Manager performs mapping of the date, time, and timestamp C data types entered in the TargetType arguments of SQLBindCol and SQLGetData or in the ValueType arguments.

I didn't copy the table here. The table shows how the DriverManager maps it ODBC 2.0 or ODBC 3.0 application from ODBC 3.x driver.

@arvind-narain
Copy link
Contributor

Thanks for the quick change RouYu. Like Selva mentioned we could have gone with 91, 92, 93 for both jdbc and odbc. We do use the same in case of SQLColumns. Can be handled separately. Currently merging this change since it does fix the current issue.

@asfgit asfgit merged commit a06dfd8 into apache:master Jul 7, 2016
@arvind-narain
Copy link
Contributor

Created JIRA for follow-up - https://issues.apache.org/jira/browse/TRAFODION-2100

@ryzuo
Copy link
Contributor Author

ryzuo commented Jul 8, 2016

Sure thing, I will handle this soon. I'll go through the driver to see if there's other place mixing the definitions (trafodion driver has been confusing the compile time and run time versioning for a long time, or we can just omit that if no one would like to use the trafodion's own odbc driver manager). Then we will have some thorough tests.

@selvaganesang
Copy link
Contributor

selvaganesang commented Jul 8, 2016

@ryzuo Does Trafodion have ts own driver manager? If so, how does ODBC application use it?

@ryzuo
Copy link
Contributor Author

ryzuo commented Jul 8, 2016

@selvaganesang Yes, there is, only Linux ODBC Driver. It provides 2 .so files: libtrafodbc_drvr64.so and libtrafodbc64.so, the first one is only the driver providing API functions, which needs a 3rd party DM like unixODBC. The second one without the _drvr suffix is the the one with a built-in DM in it. These 2 .so files shall not be used at the same time. It only "manages" its own driver, handles the data source configuration, mapping/loading the driver API functions, definitions, etc.
If users want their apps to use trafodion native DM, only -ltrafodion64 is needed to link the second .so file, and TRAFDSN will be the dsn configuration file it uses. Otherwise, with the first one, app needs to use 3rd party DM, -lodbc for unixODBC for instance, then the dsn configuration file shall be odbc.ini (prerequisite, odbcinst.ini shall be configured to locate trafodion dirver .so file).

@selvaganesang
Copy link
Contributor

In that case, it is needed to ensure that the DM libtrafodbc64.so does the mapping correctly. Looking at the code of this DM, it is not clear to me what was the need for shipping this DM instead of using the 3rd party open source DMs in unix. This DM seems to be specific to Trafodion driver alone. If we can't ascertain the need for this DM, it might be better to avoid using this DM because the ODBC application needs to be re-linked to use this DM

@ryzuo ryzuo deleted the jira306 branch July 19, 2016 03:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
5 participants