You are viewing a plain text version of this content. The canonical link for it is here.
Posted to codereview@trafodion.apache.org by xwq <gi...@git.apache.org> on 2016/04/13 10:49:48 UTC
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
GitHub user xwq opened a pull request:
https://github.com/apache/incubator-trafodion/pull/430
[TRAFODION-1686][MTDCS] ODBC: SQLParamdata ran into error for date ty…
…pe columns
Windows ODBC need the maxLen is exactly equal to the real length. So
update the maxLen in jdbc t2.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/xwq/incubator-trafodion TRAFODION-1686
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/incubator-trafodion/pull/430.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #430
----
commit f7f22aae36ecf7935912b475a1803ba8852f68e8
Author: Weiqing Xu <we...@esgyn.cn>
Date: 2016-04-13T08:38:18Z
[TRAFODION-1686][MTDCS] ODBC: SQLParamdata ran into error for date type columns
Windows ODBC need the maxLen is exactly equal to the real length. So
update the maxLen in jdbc t2.
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by xwq <gi...@git.apache.org>.
Github user xwq commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59675519
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
switch (SQLItemDesc->datetimeCode)
{
case SQLDTCODE_DATE:
SQLItemDesc->ODBCDataType = SQL_TYPE_DATE;
+ // For DATE, update the maxLen since it only have YYYY/MM/DD
+ SQLItemDesc->maxLen = 10;
break;
case SQLDTCODE_TIME:
SQLItemDesc->ODBCDataType = SQL_TYPE_TIME;
--- End diff --
Maybe we need update all the maxLen for different datetime type. However I need write some unit test case to make it work well for each datetime type.
So, for this PR, I only update SQLDTCODE_DATE for this issue .
If you think we need update all the maxLen, we can keep this PR here , I will take some days to test it and update this PR later.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59881992
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
--- End diff --
No need to use the internal format because SQLCLI accepts TIMESTAMP, DATE and TIME in character format. I think the point I am trying to make here is SQLItemDesc->maxLen isused by getMemoryAllocInfo to allocate memory buffer accordingly on the client side to be shipped to the JNI side. If so, I am curious how the parameters for date/time/timestamp worked till now?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by selvaganesang <gi...@git.apache.org>.
Github user selvaganesang commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59812021
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
--- End diff --
SQLItemDesc->maxLen is used as input for some of the data types. Does this mean it is invalid for SQLItemDesc for date columns. If so, you might consider fixing where SQLItemDesc is obtained from SQL.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by xwq <gi...@git.apache.org>.
Github user xwq commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59959894
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
--- End diff --
The issue is not about getMemoryAllocInfo here. Since the maxLen is hard code to 26, so the length of the memory is always enough.
When use MT-DCS, the maxLen will be passed to driver side. ODBC driver will check if the maxLen is equal to the real length of the value, for example , the maxLen of DATE should be 10.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by xwq <gi...@git.apache.org>.
Github user xwq commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59845709
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
--- End diff --
JDBC T2 use VARCHAR to handle DATE type.So , we need set the maxLen in T2.
Maybe a better fix is using the binary time format. But I need more time to investigate how to use binary date format from SQLCLI . So I want to submit this little fix to make the MT-DCS can run with windows ODBC.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59570974
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
switch (SQLItemDesc->datetimeCode)
{
case SQLDTCODE_DATE:
SQLItemDesc->ODBCDataType = SQL_TYPE_DATE;
+ // For DATE, update the maxLen since it only have YYYY/MM/DD
+ SQLItemDesc->maxLen = 10;
break;
case SQLDTCODE_TIME:
SQLItemDesc->ODBCDataType = SQL_TYPE_TIME;
--- End diff --
Should we add code to calculate maxLen for TIME as well? (It would be just HH:MM:SS)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by xwq <gi...@git.apache.org>.
Github user xwq commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59959933
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
switch (SQLItemDesc->datetimeCode)
{
case SQLDTCODE_DATE:
SQLItemDesc->ODBCDataType = SQL_TYPE_DATE;
+ // For DATE, update the maxLen since it only have YYYY/MM/DD
+ SQLItemDesc->maxLen = 10;
break;
case SQLDTCODE_TIME:
SQLItemDesc->ODBCDataType = SQL_TYPE_TIME;
--- End diff --
I will create a new JIRA after this PR is merged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/incubator-trafodion/pull/430
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] incubator-trafodion pull request: [TRAFODION-1686][MTDCS] ODBC: SQ...
Posted by DaveBirdsall <gi...@git.apache.org>.
Github user DaveBirdsall commented on a diff in the pull request:
https://github.com/apache/incubator-trafodion/pull/430#discussion_r59739257
--- Diff: core/conn/jdbc_type2/native/SqlInterface.cpp ---
@@ -246,10 +246,14 @@ SQLRETURN GetJDBCValues( SQLItemDesc_def *SQLItemDesc,
SQLItemDesc->signType = TRUE;
break;
case SQLTYPECODE_DATETIME:
+ // Calculate the length based on YYYY/MM/DD HH:MM:SS.ffffff
+ SQLItemDesc->maxLen = 20 + 6;
switch (SQLItemDesc->datetimeCode)
{
case SQLDTCODE_DATE:
SQLItemDesc->ODBCDataType = SQL_TYPE_DATE;
+ // For DATE, update the maxLen since it only have YYYY/MM/DD
+ SQLItemDesc->maxLen = 10;
break;
case SQLDTCODE_TIME:
SQLItemDesc->ODBCDataType = SQL_TYPE_TIME;
--- End diff --
Doing it later in a separate PR is fine. Might be good to write a separate JIRA to keep track of it.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---