Bug 81430 - libreoffice-mysql-connector cannot create/edit tables without primary key
Summary: libreoffice-mysql-connector cannot create/edit tables without primary key
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.3.0.2 rc
Hardware: x86-64 (AMD64) Linux (All)
: low enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:7.4.0
Keywords:
Depends on:
Blocks: Database-Connectivity
  Show dependency treegraph
 
Reported: 2014-07-16 14:29 UTC by Y
Modified: 2022-01-15 13:42 UTC (History)
4 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 Y 2014-07-16 14:29:33 UTC
Testing with Xubuntu 14.04 and with LibreOffice version 1:4.3.0~rc2-1ubuntu1~trusty0 from the pre-releases PPA.

Server Side:
------------
> CREATE DATABASE test;
> GRANT ALL PRIVILEGES ON test.* to 'dbuser'@'localhost' IDENTIFIED BY 'password';
> FLUSH PRIVILEGES;

dbuser performs all operations on databse test as expected.

Client Side:
------------
SSH tunnel established with:
$ ssh -L5000:server.example.com:3306 user@server.example.com

Manual connection test works well:
$ mysql -h 127.0.0.1 --port=5000 -u dbuser - p

dbuser performs all operations on databse test as expected.

Install and restart libreoffice:
$ sudo apt-get install libreoffice-mysql-connector

In LibreOffice, when I try to create a table, it reports the following error:
  SQL Status: 42000
  Error code: 1142
  CREATE command denied to user 'dbuser'@'localhost' for table 'Capacity'

A quick search for the error codes indicates that I am logged into mysql with a default null user with just about zero privileges.

I can read table content, but I cannot edit them in any meaningful way.

Workaround (JDBC):
------------------

Install and restart libreoffice:
$ sudo apt-get install libmysql-java

Configure:
* Tools -> Options -> LibreOffice -> Advanced
* tick Use a jre
** Select Oracle 1.7.0_55
* Click Class Path
** Add Archive /usr/share/java/mysql-connector-java.jar
* Restart LO (or maybe reboot, at first it did not work)

Now LibreOffice Base can edit a MySQL database over an SSH tunnel.

Bug submitted also with the packager as I am not sure where the problem lies:
https://bugs.launchpad.net/ubuntu/+source/libreoffice/+bug/1342755
Comment 1 Y 2014-07-16 15:08:55 UTC
More research into this:
* I can create tables from LibreOffice to MySQL, but they must have primary keys
* Once tables have primary keys, they can be edited
* Table created MySQL-side, that works:
> CREATE TABLE idxtest (a INT NOT NULL AUTO_INCREMENT PRIMARY KEY, b VARCHAR(10));
* Table that did not work, making it work:
> ALTER TABLE test ADD d INT UNSIGNED NOT NULL AUTO_INCREMENT, ADD PRIMARY KEY (d);

Is there a reason for LibreOffice Base not to allow writing into non-indexed tables?
Comment 2 Alex Thurgood 2014-07-18 09:59:10 UTC
(In reply to comment #1)

> 
> Is there a reason for LibreOffice Base not to allow writing into non-indexed
> tables?

Other than that it has always been this way with the C++ native connector, not that I know of. There is probably some discussion about this in a similarly, closed, previously filed bug report, but I don't remember where.
Comment 3 Alex Thurgood 2014-07-18 10:05:54 UTC
Note also that the connector itslef is a non-bundled extension, i.e. a kind of external project, despite having hooks in the code tree. It is not built by default with the application code, but rather it is the distribs that build the connector as a separate add-on and then provide it in their packaging systems, so in order for this to change, it would require someone to write handling code that would allow non-indexed fields to be writable via the connector. I don't think that this is as trivial to achieve as it might first seem (although I'm not a programmer, so can not estimate the effort required).

At best, this is a RFE, but whether there will ever be any takers to actually do the work is another matter.
Comment 4 Alex Thurgood 2015-01-03 17:38:46 UTC Comment hidden (no-value)
Comment 5 Julien Nabet 2022-01-04 21:27:42 UTC
Just for the record, the readonly is set at this point:
dbaccess/source/ui/control/FieldDescControl.cxx:988
    987     // Enable/disable Controls
    988     bool bRead(IsReadOnly());
    989 
    990     SetReadOnly( bRead );
    991 }


Now why IsReadOnly() returns true only for Mysql direct connector is another story.
Comment 6 Julien Nabet 2022-01-06 18:56:34 UTC
The situation is worse now with Mysql (rather Mariadb) direct connector, we can't modify too tables with a primary key.

Alex/Lionel/Robert: I noticed this also with LO Debian package 7.2.4.

With MysqlJDBC, no pb to change tables (with or without PK) with master sources updated today.
Comment 7 Julien Nabet 2022-01-06 22:16:13 UTC
It seems methods about Table/Tables are lacking.

Here specifically, it lacks alterColumnByName method
Comment 8 Robert Großkopf 2022-01-07 07:46:30 UTC
I have tested this one: 
Opened a Base file with direct connection to a MariaDB. 
Created a table without primary key.
A message appears (something like: Should a primary key be created. You won't be able to input data …)
I won't create the table with the primary key, so it will be created without a primary key.
I could insert rows in the table by Tools → SQL, but I can't insert rows by the GUI in table view.

Changing the table after it has been created: Works here without any problems. Only creating a primary key after creating a table won't work in the GUI.

Note: 
The original post is made for LO 4.3. The direct connection for MySQL/MariaDB has been included into LO since LO 6.2. So the behavior will be different to the original post now.
I also couldn't input values in this table through GUI when connecting to MariaDB through JDBC with org.mariadb.jdbc.Driver.

All tested with LO 7.3.0.1 on OpenSUSE 15.3 64bit rpm Linux
Comment 9 Julien Nabet 2022-01-07 20:36:13 UTC
(In reply to Julien Nabet from comment #7)
> It seems methods about Table/Tables are lacking.
> 
> Here specifically, it lacks alterColumnByName method

I added lots of file locally to try to implement it and this method  dbaui::OTableController::isAlterAllowed (this=0x3ffffb0) at dbaccess/source/ui/tabledesign/TableController.cxx:1385
1385	    bool bAllowed(!m_xTable.is() || Reference<XAlterTable>(m_xTable,UNO_QUERY).is())

I runned make connectivity.clean && make && make postprocess + reset profile, this method still returns false.

I must have missed something but don't know for the moment.
Comment 10 Julien Nabet 2022-01-07 21:47:15 UTC
Lionel: any idea what I could have missed?
Comment 11 Julien Nabet 2022-01-08 09:32:49 UTC
I'm comparing with Firebird part to find the missing point.

Should XTablesSupplier.idl and/or XDataDefinitionSupplier.idl be implemented so XAlterTable.idl works or unrelated?
Comment 12 Julien Nabet 2022-01-08 18:28:08 UTC
At last, I finally succeeded at making the fields of table not readonly (pk or not pk case)
There's still some work (at least really implementing alterColumnByName) but I had already to add a lot of things.
So I'd like first to split it in several patches but I must find the right way to avoid breaking something.
Comment 13 Commit Notification 2022-01-14 20:41:59 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/628fedabdd27ad08e9a42f47106864751e493c2c

Mysql/MariaDB: tdf#81430 make tables editable (with or without PK)

It will be available in 7.4.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 14 Julien Nabet 2022-01-14 21:00:46 UTC
Here are some infos about work done here.
There have been several patches:

1) https://cgit.freedesktop.org/libreoffice/core/commit/?id=dcb7bd026cc1813eaee8b7dadba0583348b4458e

wasn't related to the bugtracker and was about removing system schemas from tables pane:
goal: facilitate tests and do like MYSQL JDBC part

2) https://cgit.freedesktop.org/libreoffice/core/commit/?id=8dd86eb5602bed539da5ffb584e09847dbde4d60
add first part of the skeleton of the whole mechanism

3) https://cgit.freedesktop.org/libreoffice/core/commit/?id=bb29b12a9e367d181a5d9d962d466df41e093e0c
add second part of the skeleton of the whole mechanism

4) https://cgit.freedesktop.org/libreoffice/core/commit/?id=628fedabdd27ad08e9a42f47106864751e493c2c
implement it + implement things manually which were done quite automatically.

Even if I tested some cases, it still needs some testing and may have brought some regressions, so I won't backport them on 7.3 branch or below.
Of course, if there are too much regressions and that I can't fix them (at least important ones), I'll revert these patches. To identify the commits quickly, I used the prefix "Mysql/MariaDB" in the title of the description of each.


Just for the record, I know that for database implementations there's the general mechanism and the manual mechanism. I don't know what's feasible with the general mechanism or when to choose one or the other. Perhaps a far more simple and therefore elegant patch would have fixed this bugtracker by using the general mechanism but since I know it too little, I used parts of Firebird implementation (manual mechanism) which were working and adapted them.
Comment 15 Julien Nabet 2022-01-15 00:11:00 UTC
I just found 2 regressions:
When creating the table with table design, the table is created but impossible to quit the table design
gdb provides:
#0  connectivity::mysqlc::Tables::createObject(rtl::OUString const&) (this=0x442bcb0, rName="Table15") at connectivity/source/drivers/mysqlc/mysqlc_tables.cxx:54
#1  0x00007efca703a5f7 in connectivity::mysqlc::Tables::appendObject(rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::beans::XPropertySet> const&)
    (this=0x442bcb0, rName="Table15", rDescriptor=uno::Reference to (dbaccess::ODBTableDecorator *) 0x4aea4f0) at connectivity/source/drivers/mysqlc/mysqlc_tables.cxx:191
#2  0x00007efcb770a9eb in connectivity::sdbcx::OCollection::appendByDescriptor(com::sun::star::uno::Reference<com::sun::star::beans::XPropertySet> const&)
    (this=0x442bcb0, descriptor=uno::Reference to (dbaccess::ODBTableDecorator *) 0x4aea4f0) at connectivity/source/sdbcx/VCollection.cxx:373
#3  0x00007efcace5be22 in dbaccess::OTableContainer::appendObject(rtl::OUString const&, com::sun::star::uno::Reference<com::sun::star::beans::XPropertySet> const&) (this=
    0x43eab90, _rForName="test.Table15", descriptor=uno::Reference to (dbaccess::ODBTableDecorator *) 0x4aea4f0) at dbaccess/source/core/api/tablecontainer.cxx:285
#4  0x00007efcb770a9eb in connectivity::sdbcx::OCollection::appendByDescriptor(com::sun::star::uno::Reference<com::sun::star::beans::XPropertySet> const&)
    (this=0x43eab90, descriptor=uno::Reference to (dbaccess::ODBTableDecorator *) 0x4aea4f0) at connectivity/source/sdbcx/VCollection.cxx:373
#5  0x00007efca8c9e4c9 in dbaui::OTableController::doSaveDoc(bool) (this=0x4623520, _bSaveAs=false) at dbaccess/source/ui/tabledesign/TableController.cxx:333

+ impossible to drop the table

great...
Comment 16 Commit Notification 2022-01-15 09:28:30 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

Mysql/MariaDB: related:tdf#81430, fix regression drop table

It will be available in 7.4.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 17 Commit Notification 2022-01-15 10:31:26 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

Mysql/MariaDB: related:tdf#81430, fix regression create table

It will be available in 7.4.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 18 Julien Nabet 2022-01-15 10:39:04 UTC
(In reply to Julien Nabet from comment #15)
> I just found 2 regressions:
> ...

The 2 regressions are now fixed. However, I'm not satisfied since they seem to be more a bandaid than a real fix.
Again, I'm quite sure I should rely more on the general mechanism instead of reimplementing (and not elegantly) large parts of code manually. If only there was some doc.
I took a look at connectivity/workben but it seems a bit light (perhaps also obsolete?)
Comment 19 Julien Nabet 2022-01-15 11:14:10 UTC
2 other regressions
1) after having created a new view, this one doesn't display automatically.
2) View, Refresh Tables doesn't work, the table/view list is empty
Of course, when closing file and reloading it, it's ok, tables and the new view appear.

What a mess...
Comment 20 Commit Notification 2022-01-15 13:17:39 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

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

Mysql/MariaDB: related:tdf#81430, fix regression refresh tables/views manually

It will be available in 7.4.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 21 Alex Thurgood 2022-01-15 13:42:14 UTC
(In reply to Commit Notification from comment #20)
> Julien Nabet committed a patch related to this issue.
> It has been pushed to "master":
> 
> Affected users are encouraged to test the fix and report feedback.

Hi Julien, thanks for taking a stab at this, quite the effort! 

From memory of exchanging with the OOo DB devs at Sun many, many years ago, the whole basic idea was indeed to copy/paste/adapt from the basic driver code, and there was little to no code commentary or documentation when the code was first released, so I'd say you're having a pretty good go! 😁