Bug 143895 - Mysql MEDIUMINT data type but interpreted correctly by Base table definition
Summary: Mysql MEDIUMINT data type but interpreted correctly by Base table definition
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
7.0.6.2 release
Hardware: All All
: medium normal
Assignee: Julien Nabet
URL:
Whiteboard: target:7.3.0 target:7.2.1 target:7.1.6
Keywords:
Depends on:
Blocks:
 
Reported: 2021-08-16 09:01 UTC by john farley
Modified: 2021-08-16 16:35 UTC (History)
2 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 john farley 2021-08-16 09:01:32 UTC
UI
I have a MySql database which I have successfully accessed via Base and successfully created a form over.
I noticed one numeric column truncates to a value of 32,767. That column in the MySql database is MEDIUMINT but Base has defaulted to SMALLINT.
If I edit the table definition in Base for that column to (what seems a valid) value of MEDIUMINT, it is not accepted.
If I edit the table definition in Base for that column to value of INTEGER, it is accepted.
Comment 1 Julien Nabet 2021-08-16 09:47:16 UTC
Lionel: I took a look at it and noticed this:
git grep -in mediumint
source/drivers/mysqlc/mysqlc_databasemetadata.cxx:426:           "LOOP, LOW_PRIORITY, MATCH, MEDIUMBLOB, MEDIUMINT,"
source/drivers/mysqlc/mysqlc_general.cxx:200:    if (sType.equalsIgnoreAsciiCase("int") || sType.equalsIgnoreAsciiCase("mediumint"))
source/drivers/mysqlc/mysqlc_general.cxx:278:            return isUnsigned ? (isZerofill ? OUString{ "MEDIUMINT UNSIGNED ZEROFILL" }
source/drivers/mysqlc/mysqlc_general.cxx:279:                                            : OUString{ "MEDIUMINT UNSIGNED" })
source/drivers/mysqlc/mysqlc_general.cxx:280:                              : OUString{ "MEDIUMINT" };
source/drivers/mysqlc/mysqlc_types.cxx:428:    // ----------- MySQL-Type: MEDIUMINT SDBC-Type: INTEGER ----------
source/drivers/mysqlc/mysqlc_types.cxx:430:        "MEDIUMINT", // Typename
source/drivers/mysqlc/mysqlc_types.cxx:442:        "MEDIUMINT", // local type name

    // ----------- MySQL-Type: MEDIUMINT SDBC-Type: INTEGER ----------
    {
        "MEDIUMINT", // Typename
        com::sun::star::sdbc::DataType::INTEGER, // sdbc-type
        7, // Precision
        "", // Literal prefix
        "", // Literal suffix
        "[(M)] [UNSIGNED] [ZEROFILL]", // Create params
        com::sun::star::sdbc::ColumnValue::NULLABLE, // nullable
        false, // case sensitive
        com::sun::star::sdbc::ColumnSearch::FULL, // searchable
        true, // unsignable
        false, // fixed_prec_scale
        true, // auto_increment
        "MEDIUMINT", // local type name
        0, // minimum scale
        0 // maximum scale
    },
See https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/mysqlc/mysqlc_types.cxx?r=1dd9200b#657


    277         case MYSQL_TYPE_INT24:
    278             return isUnsigned ? (isZerofill ? OUString{ "MEDIUMINT UNSIGNED ZEROFILL" }
    279                                             : OUString{ "MEDIUMINT UNSIGNED" })
    280                               : OUString{ "MEDIUMINT" };

See https://opengrok.libreoffice.org/xref/core/connectivity/source/drivers/mysqlc/mysqlc_general.cxx?r=c558ad61#277

So I thought about this patch:
diff --git a/connectivity/source/drivers/mysqlc/mysqlc_general.cxx b/connectivity/source/drivers/mysqlc/mysqlc_general.cxx
index 7ed11fe3ff13..878efdc3be24 100644
--- a/connectivity/source/drivers/mysqlc/mysqlc_general.cxx
+++ b/connectivity/source/drivers/mysqlc/mysqlc_general.cxx
@@ -193,11 +193,11 @@ sal_Int32 mysqlStrToOOOType(const OUString& sType)
     // TODO other types.
     if (sType.equalsIgnoreAsciiCase("tiny") || sType.equalsIgnoreAsciiCase("tinyint"))
         return css::sdbc::DataType::TINYINT;
-    if (sType.equalsIgnoreAsciiCase("smallint") || sType.equalsIgnoreAsciiCase("mediumint"))
+    if (sType.equalsIgnoreAsciiCase("smallint"))
         return css::sdbc::DataType::SMALLINT;
     if (sType.equalsIgnoreAsciiCase("longtext"))
         return css::sdbc::DataType::LONGVARCHAR;
-    if (sType.equalsIgnoreAsciiCase("int"))
+    if (sType.equalsIgnoreAsciiCase("int") || sType.equalsIgnoreAsciiCase("mediumint"))
         return css::sdbc::DataType::INTEGER;
     if (sType.equalsIgnoreAsciiCase("varchar") || sType.equalsIgnoreAsciiCase("set")
         || sType.equalsIgnoreAsciiCase("enum"))

I'm not sure if it's sufficient but I think it may help.

Also Mysql has 5 int and derived types:
https://dev.mysql.com/doc/refman/8.0/en/integer-types.html
TINYINT
SMALLINT
MEDIUMINT
INT
BIGINT

and LO has only 4:
https://opengrok.libreoffice.org/xref/core/offapi/com/sun/star/sdbc/DataType.idl?r=2b383d19
TINYINT
SMALLINT
INT
BIGINT

Not sure if the patch may have wrong side effect.
Comment 2 Lionel Elie Mamane 2021-08-16 10:22:54 UTC
(In reply to Julien Nabet from comment #1)
> So I thought about this patch:

Looks good.

> I'm not sure if it's sufficient but I think it may help.

Yes.
Comment 3 Julien Nabet 2021-08-16 10:48:57 UTC
https://gerrit.libreoffice.org/c/core/+/120536
Comment 4 Commit Notification 2021-08-16 13:57:23 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/997ff7166ceca0a5af80297a0e789af2ff0c6617

Related tdf#143895: Mysql MEDIUMINT is DataType::INTEGER not DataType::SMALLINT

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 5 Julien Nabet 2021-08-16 14:03:27 UTC
I won't put it at FIXED for the moment, since I'm not sure it'll solve the pb.

However, I submitted the patch on 7.2 (https://gerrit.libreoffice.org/c/core/+/120447) and 7.1 (https://gerrit.libreoffice.org/c/core/+/120448) branches.

John: on which env are you? Would it be possible you give a try with a daily build (see https://dev-builds.libreoffice.org/daily/master/) in 24/48 hours, depending on which build the fix will be?
Comment 6 Julien Nabet 2021-08-16 15:06:51 UTC
I could create a MariaDB (10.5.12) table with mediumint, it works now.
I could reproduce the pb by reverting the patch locally.

I've just noticed, I can't edit table field with master sources (all fields are disabled) but could with LO Debian package 7.0.4 but that's another story.
Anyway, let's put this one to fixed.
Comment 7 Commit Notification 2021-08-16 15:55:11 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-2":

https://git.libreoffice.org/core/commit/5b6c922226049d32475a249b0df184955aaa2747

Related tdf#143895: Mysql MEDIUMINT is DataType::INTEGER not DataType::SMALLINT

It will be available in 7.2.1.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2021-08-16 16:35:41 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

https://git.libreoffice.org/core/commit/b04a0ddd3ca8f5d1b1b38208ef59fb27321378b8

Related tdf#143895: Mysql MEDIUMINT is DataType::INTEGER not DataType::SMALLINT

It will be available in 7.1.6.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.