s390x support: changes to fix test failures #43
Conversation
lawrinn
force-pushed
the
master
branch
2 times, most recently
from
c086cfd
to
fcb9a5f
Compare
|
Thank you for your contribution Similar to other open source projects, the MariaDB Corporation needs to have shared ownership of all code that is included in the MariaDB C/ODBC distribution. The easiest way to achieve this is by submitting your code under the BSD-new license. Please indicate in a comment below that you are contributing your new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license. We cannot test on s390, this platform is not officially supported by us, thus we cannot guarantee, that your change won't be broken by some newer changes. Since your changes are in tests only, I can imagine that wouldn't be a great concern for you. If you are fine with all that, than I'll take another look, maybe couple of changes would be needed. |
|
I am contributing my new code of the whole pull request, including one or several files that are either new files or modified ones, under the BSD-new license. |
|
@lawrinn Please review changes and let me know in case of any modifications |
lawrinn
force-pushed
the
master
branch
3 times, most recently
from
c65037c
to
fe23856
Compare
| @@ -586,7 +586,11 @@ SQLRETURN MADB_DbcConnectDB(MADB_Dbc *Connection, | |||
| MADB_Dsn *Dsn) | |||
| { | |||
| char StmtStr[128]; | |||
| #if defined(__s390x__) | |||
There was a problem hiding this comment.
It can be just changed to my_bool without if defined. Or just initialized with someting like 0x01010101
| @@ -70,8 +70,11 @@ ODBC_TEST(test_CONO3) | |||
| ODBC_TEST(simple_test) | |||
| { | |||
| SQLRETURN rc= SQL_SUCCESS; | |||
|
|
|||
| #if defined(__s390x__) | |||
| SQLSMALLINT value=3; | |||
There was a problem hiding this comment.
Same - just can be changed to SQLSMALLINT
| @@ -1528,7 +1531,12 @@ ODBC_TEST(odbc45) | |||
| for (i= 0; i<sizeof(XpctdValue); ++i) | |||
| { | |||
| CHECK_STMT_RC(Stmt, SQLFetch(Stmt)); | |||
| #if defined(__s390x__) | |||
| SQLGetData(Stmt, 1, SQL_C_BIT, &value, sizeof(value), 0); | |||
There was a problem hiding this comment.
It doesn't look, that for test it's important to fetch values as ints, thus it can be just changed to fetch values as SQL_C_BIT
| #if defined(__s390x__) | ||
| SQLGetData(Stmt, 1, SQL_C_BIT, &value, sizeof(value), 0); | ||
| is_num(value, XpctdValue[i]); | ||
| #else |
There was a problem hiding this comment.
I also suspect, that explicit casting of XpctdValue[i] to int could fix the test for s390. Not sure, though
|
I've merged the PR as is, but I'm gonna make the behavior you added #if defined(s390x) a default behavior for all platforms. Just like I said in my comments |
Thanks for merging the PR. You will be raising another PR to make a default behavior? |
|
I've done that already. I have a question, though. Or a request even. I have two open JIRA tickets re s390 - https://jira.mariadb.org/browse/ODBC-130 and https://jira.mariadb.org/browse/ODBC-128 Do you happen to know, if they can be closed? |
|
@lawrinn Yes. These JIRA tickets can be closed as we do not see this failure now. |
Code changes required to fix the below test failures on s390x
1 - odbc_basic
20 - odbc_error
21 - odbc_param
Please review.