Bug Hunting Session
Bug 88601 - LO BASE: Failure to correctly interpret range for UNSIGNED integers from MySQL database (jdbc and native connectors) [summary in comment 9]
Summary: LO BASE: Failure to correctly interpret range for UNSIGNED integers from MySQ...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.3.5.2 release
Hardware: Other All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: difficultyInteresting, easyHack, skillCpp, skillJava, skillSql
Depends on:
Blocks: Database-Connectivity
  Show dependency treegraph
 
Reported: 2015-01-19 21:42 UTC by Doug
Modified: 2018-09-23 11:10 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Doug 2015-01-19 21:42:31 UTC
SHORT DESCRIPTION:  LO Base uses incorrect range of values for UNSIGNED integers in MySQL/MariaDB tables.  LO and/or its connectors treat UNSIGNED integer fields as if they are SIGNED and resulting in unexpected/erroenous outcomes.  Erroneous functionality regardless of connector (two jdbc connectors tested, along with direct LO native connector), so reporting here.

DETAILED DESCRIPION:

1.  Create MySQL/MariaDB database with Table1;
2.  On Table1 create Field1 as UNSIGNED TinyInt;

NOTE:  UNSIGNED TinyInt has range 0 to 255.

3.  Connect to MySQL database using any method with LO Base; and,
4.  In table, attempt to enter value into Field1 of 128 to 255.  (these values can successfully be entered directly into MySQL field without LO)

UNEXPECTED RESULT:

JBDC Drivers (com.mysql.jdbc.Driver and org.mariadb.jdbc.Driver)
"Error Updating Record // Data truncation:  Out of range value for column 'Field1' at row 1"

"SQL Status: 22001 // Error code: 1264"

Also received 22003.

MySQL Native Connector did not return error, simply (erroenously) changed out-of-range value between 128 and 255 to 0 and saved.

ALTERNATIVELY:
At step 4. substitute a value of -1 to -128.

ALTERNATIVE UNEXPECTED RESULT:  On loss of focus, value will change to the product of the following equation:

NewValue = 256 + TypedValue

That is, a value of negative one will change to a NewValue of 255.

Then, as noted above, attempting to save record will result in error above for jdbc drivers.  Native connector just changes the value and saves, usually the value 0.

NOTE ALSO:
If values 128 to 255 already are stored in Field1, they will be correctly displayed in the table view.  However, they will not be displayed correctly on a form in a numeric field.  The form view just displays zero.

PRESENT WORKAROUND is to use bigger integer types so that I do not encounter this bug.  However, other UNSIGNED integer types also appear to be affected by this bug, just need bigger numbers to encounter them.  

Unable to test on odbc connector.  

Attempted to test on HSQLDB Embedded (LO type) but it does not support unsigned integer types, which is the difference that presumably resulted in this bug.
Comment 1 Alex Thurgood 2015-01-20 15:42:36 UTC
@Doug : if the connectors are the root cause (I'm not saying they are), this will not be a LO Base bug.
Comment 2 Alex Thurgood 2015-01-20 16:02:05 UTC
Can not reproduce with 

Version: 4.3.5.2
Build ID: 3a87456aaa6a95c63eea1c1b3201acedf0751bd5

on OSX 10.10.1 connecting to a remote mysql instance 

using MySQL Connector C++ extension 1.02

CREATE TABLE `Table1` (
  `Field1` tinyint(1) unsigned NOT NULL,
  `Field2` int(10) unsigned NOT NULL AUTO_INCREMENT,
  PRIMARY KEY (`Field2`)
) ENGINE=MyISAM AUTO_INCREMENT=10 DEFAULT CHARSET=latin1 |
Comment 3 Alex Thurgood 2015-01-20 16:12:38 UTC
However, with JDBC connector mysql-connector-java-5.1.21, I get a completely different problme, namely that my tinyint(1) field shows up as a Boolean in table grid view, with all boxes ticked.

Opening up a separate report for this.

FWIW, here are the table values I could enter with the native mysql connector extension as displayed from mysql cli :

mysql> select * from Table1;
+--------+--------+
| Field1 | Field2 |
+--------+--------+
|    244 |      1 |
|    255 |      2 |
|    213 |      3 |
|    128 |      4 |
|      1 |      5 |
|      2 |      6 |
|      4 |      7 |
|      5 |      8 |
|    255 |      9 |
+--------+--------+
9 rows in set (0,00 sec)
Comment 4 Doug 2015-01-20 16:49:20 UTC
(In reply to Alex Thurgood from comment #1)
> @Doug : if the connectors are the root cause (I'm not saying they are), this
> will not be a LO Base bug.

Reported here because all connectors working/available to me generated same problem:

mysql-connector-java-5.1.34
mariadb-java-client-1.1.8
LO MySQL Connector 1.0.2 (native)

Sounding from Alex @2 like connectors are the problem, if so, I may cross-report.  Thx.
Comment 5 Alex Thurgood 2015-01-20 17:24:51 UTC
FWIW, see also Lionel's comments in my report for bug 88631
Comment 6 Lionel Elie Mamane 2015-01-20 17:43:08 UTC
The SQL standard, ODBC, JDBC (and I think Java in general), SDBC (LibreOffice) have no notion of unsigned integer data types for column types. "UNSIGNED" is a MySQL-specific non-standard extension.

I'm not opposed on principle to extending LibreOffice's native MySQL driver to handle MySQL's unsigned integer datatyeps, *if* it is possible in a not too invasive way in the rest of code. I'm not sure if it is possible, at least in a clean way.

Abandon any hope of having this work entirely correctly through ODBC or JDBC, unless you e.g. map "unsigned short" to "signed integer", "unsigned integer" to "signed long", etc.

In other words, I take patches.
Comment 7 Lionel Elie Mamane 2015-01-20 18:18:35 UTC
Since the JDBC drivers (by necessity) shows us the datatypes as signed, I'm going to set this as "NOTOURBUG" for the JDBC part.

The behaviour with the native driver of setting 0 seems to be the behaviour of MySQL, since LibreOffice (as noted later in the bug) will send the value as a negative value:

mysql> CREATE TEMPORARY TABLE F (i tinyint unsigned);
Query OK, 0 rows affected (0.03 sec)

mysql> INSERT INTO F VALUES ((-5));
Query OK, 1 row affected, 1 warning (0.01 sec)

mysql> SELECT * FROM F;
+------+
| i    |
+------+
|    0 |
+------+
1 row in set (0.00 sec)

As such, also NOTOURBUG.

As to the behaviour of mapping negative value "v" to 256+v, that's how casting from signed to unsigned happens in C/C++... Maybe that could be improved in LibreOffice <shrug> You can fork it into its own bug entry if you want.
Comment 8 Doug 2015-07-07 13:59:19 UTC
Cross-reported to MariaDB team which respond as follows:

* * *

"NOTOURBUG seems wrong. [LibreOffice] should be using the integer type, which is wide enough to incorporate 0-255. Like, short, or int, or if they wish even long.
They can consult http://dev.mysql.com/doc/connector-j/en/connector-j-reference-type-conversions.html to figure out how unsigned types are mapped to correct java types. MariaDB driver uses the same type conversion as ConnectorJ "

* * *

I also experienced this type-conversion error on MySQL ODBC connector (in addition to the others noted earlier).  The common thread is Base's handling of UNSIGNED integers from MySQL/MariaDB.  I have not had the opportunity to look 'under the hood', but in the circumstances I think the MariaDB team is right that this bug should be reopened in LibreOffice Base.

I am not sure the MariaDB team's solution is exactly right, as it would require there always be a Base integer variable that is larger than the MySQL integer variable-- I expect their solution will break down with an UNSIGNED BIGINT.
Comment 9 Lionel Elie Mamane 2015-07-07 17:53:39 UTC
Now that I look again at it, it requires some reworking of LibreOffice internals. Definitely not an introductory EasyHack, but can be done with time and good way to discover LibreOffice internals. I take patches.

The callchain is:

#0  dbtools::setObjectWithInfo (_xParams=uno::Reference to (dbaccess::OPreparedStatement *) 0x3a1ab20, 
    parameterIndex=parameterIndex@entry=1, _rValue=..., sqlType=sqlType@entry=-6, scale=scale@entry=0)
    at connectivity/source/commontools/dbtools.cxx:1900
#1  0x00007f41123b4e71 in dbaccess::OCacheSet::setParameter (nPos=nPos@entry=1, 
    _xParameter=uno::Reference to (dbaccess::OPreparedStatement *) 0x3a1ab20, _rValue=..., _nType=_nType@entry=-6, 
    _nScale=_nScale@entry=0)
    at dbaccess/source/core/api/CacheSet.cxx:382
#2  0x00007f41123f41cf in dbaccess::OKeySet::executeUpdate (this=this@entry=0x3616220, 
    _rInsertRow=rtl::Reference to 0x3a170c0, _rOriginalRow=rtl::Reference to 0x3a5a910, 
    i_sSQL="UPDATE `fdo85190`.`action_type` SET `action_type_id` = ? WHERE `action_type_id` = ?", i_sTableName="", 
    _aIndexColumnPositions=std::__debug::vector of length 0, capacity 0)
    at dbaccess/source/core/api/KeySet.cxx:682
#3  0x00007f41123f3a29 in dbaccess::OKeySet::updateRow (this=0x3616220, _rInsertRow=rtl::Reference to 0x3a170c0, 
    _rOriginalRow=rtl::Reference to 0x3a5a910, _xTable=uno::Reference to (dbaccess::ODBTableDecorator *) 0x321d760)
    at dbaccess/source/core/api/KeySet.cxx:653
#4  0x00007f411249688f in dbaccess::ORowSetCache::updateRow (this=0x3971ce0, _rUpdateRow=rtl::Reference to 0x3a170c0, 
    o_aBookmarks=std::__debug::vector of length 0, capacity 0)
    at dbaccess/source/core/api/RowSetCache.cxx:1358
#5  0x00007f411245b0fa in dbaccess::ORowSet::updateRow (this=0x3ae3210)
    at dbaccess/source/core/api/RowSet.cxx:952


Taking it bottom-up, here's what needs to happen:

1) dbtools::setObjectWithInfo needs an extra boolean parameter "isSigned".
   (file connectivity/source/commontools/dbtools.cxx)

   it uses that parameter (and not only _rValue.isSigned()) to decide what to call on _xParams

2) the same for dbaccess::OCacheSet::setParameter
   (file dbaccess/source/core/api/CacheSet.cxx)

3) the class/struct dbaccess::SelectColumnDescription needs an extra field "bSigned",
   which needs to be populated in dbaccess::getColumnPositions
   (file dbaccess/source/core/api/KeySet.cxx)
   and possibly other places

   This may need to add a new property "isSigned" to table and query columns,
   and actually change the service com::sun::star::sdbcx::Column to include that new property.
   This part makes this work *completely* not introductory. Contact me if you are interested
   in doing it.

4) dbaccess::OKeySet::executeUpdate
   (and possibly other places)

   use that new field to provide the isSigned parameter of dbaccess::OCacheSet::setParameter
Comment 10 Buovjaga 2015-10-09 18:21:35 UTC
Based on comment 9, status should be NEW.
Comment 11 Robinson Tryon (qubit) 2015-12-13 09:55:57 UTC Comment hidden (obsolete)
Comment 12 Robinson Tryon (qubit) 2016-02-18 14:51:54 UTC Comment hidden (obsolete)
Comment 13 tmonk 2018-05-12 18:20:32 UTC
Since this post is reported in 2015-Jan,
I wonder how is the test result in latest environment.

Test Environment:
OS: Ubuntu 16.04 LTS

libreoffice 5.4.6.2 (2018-05-15)
Build ID: 4014ce260a04f1026ba855d3b8d91541c224eab8

mysql Distrib 5.7.22 (2018-01-15 General Availability)

mysql-connector-java-8.0.11.jar (2018-04-18)

Test Table:

CREATE TABLE Table1 (
Field1 tinyint unsigned NOT NULL,
PRIMARY KEY (Field1) )

Detail:
Insert row from LO-base.  No problem with value from 0 to 255.
If I trying to insert value outside of 0 to 255.
L0-base will block you with error message.

But if I trying to reopen this table when there is a row with value from 128 to 255, the table will fail to load with the following error message :
" The data content could not be loaded.
'234' in column '1' is outside valid range for the datatype TINYINT."

This only happened when this column is set as UNSINGED and PRIMARY KEY.

This is my very first comment in Bugzilla.
Any advice is welcome.
Comment 14 Lionel Elie Mamane 2018-05-14 07:55:27 UTC
(In reply to tmonk from comment #13)
> Since this post is reported in 2015-Jan,
> I wonder how is the test result in latest environment.
> (...)
> This only happened when this column is set as UNSINGED and PRIMARY KEY.

Thanks for your courage.

Did you try testing with non-PRIMARY KEY unsigned integer type
columns? Then following Doug's reproduction instructions? What was the
result?
Comment 15 tmonk 2018-05-14 17:05:15 UTC
(In reply to Lionel Elie Mamane from comment #14)

For non-PRIMARY KEY UNSIGNED integer column, I failed to reproduce Dong's unexpected result at step 4.

>4.  In table, attempt to enter value into Field1 of 128 to 255.
>(these values can successfully be entered directly into MySQL field without LO)

When I inserted values from 2^31 ~ 2^32-1, LO-base inserted to table successfully. No error when reopen the table.

I also tried PRIMARY KEY UNSIGNED integer column and PRIMARY KEY UNSIGNED smallint column. LO-base will fail to reopen table when there is a value from 2^31 ~ 2^32-1 ( 2^14 ~ 2^15-1 for smallint ) in the column. Similar with the situation I reported at Comment 13.