Bug 70144 - embedded Firebird db disables "Save", loses some changes
Summary: embedded Firebird db disables "Save", loses some changes
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
4.2.0.0.alpha0+ Master
Hardware: Other All
: medium blocker
Assignee: Not Assigned
URL:
Whiteboard: target:4.2.0 target:4.3.0
Keywords:
Depends on:
Blocks:
 
Reported: 2013-10-04 16:37 UTC by Terrence Enger
Modified: 2013-12-04 19:51 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
starting point for STR (43.08 KB, application/vnd.oasis.opendocument.base)
2013-10-04 16:37 UTC, Terrence Enger
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Terrence Enger 2013-10-04 16:37:26 UTC
Created attachment 87131 [details]
starting point for STR

This reminds me of a bug report I have seen recently, but I am unable
to find it.  Redirection welcome.


STR:

( 1) Open the attached example.odb.

( 2) If LibreOffice issues a security warning about macros in the
     document, click <Disable Macros>.

( 3) Open table Table1.  Program presents Table Data View showing
     three rows of data; the Save icon in the toolbar is greyed out.

( 4) In the empty row of the data grid, type 4<tab>four.  The Save
     icon in the toolbar is enabled.

( 5) Type <tab>.  Cursor moves in a new empty row, and the Save icon
     in the toolbar is greyed out.

( 6) Close Table Data View. Program returns focus to example.odb; the
     save icon in the toolbar is greyed out.

( 7) Close example.odb.  Program display Start Center.

( 8) From the list of recently used files, open example.odb.

( 9) If LibreOffice issues a security warning about macros in the
     document, click <Disable Macros>.

(10) Open table Table1.

     Program action expected: to display Table Data View with four
     rows of data.

     Program action observed: to display Table Data View with three
     rows of data.
     

This observation comes from master commit 95e95e0, fetched 2013-09-19,
configured with

    --enable-option-checking=fatal
    --enable-dbgutil
    --enable-crashdump
    --without-system-postgresql
    --without-myspell-dicts
    --with-extra-buildid
    --without-doxygen
    --with-external-tar=/home/terry/lo_hacking/git/src

built on ubuntu-natty (11.04) 32-bit, executing on ubuntu-quantal
(12.10) 64-bit.
Comment 1 m.a.riosv 2013-10-04 21:32:24 UTC
Reproducible.
Win7x64 Ultimate
Version: 4.2.0.0.alpha0+ Build ID: 0d0d3a0540dad2d0f417e21df1183dfc33964357
TinderBox: Win-x86@39, Branch:master, Time: 2013-09-30_23:58:26
Comment 2 Andrzej Hunt 2013-10-06 15:04:53 UTC
I'd be interested to know what happens with other databases. In the case of HSQLDB all changes are stored directly to file due to the way the HSQLDB/.odb interaction works (and apparently HSQLDB lacks proper transaction support) -- with external databases I suspect nothing should happen until a transaction commited (as is the case with Firebird).

I suspect the problem is simply that "Save" is erronously not enabled when we add a row, i.e. we simply have to set the Modified flag when inserting data too (as happens for most other operations). (This might be a historical artifact of Base having been tested mainly with HSQLDB?)

That would however mean that in Firebird data is only ever commited to the .odb file with a "Save", whereas HSQLDB always has its data stored immediately, i.e. there'd be a significant difference between the operation of the two dbs in practice.
Comment 3 Lionel Elie Mamane 2013-10-06 15:15:41 UTC
(In reply to comment #2)

> I suspect the problem is simply that "Save" is erronously not enabled when
> we add a row, i.e. we simply have to set the Modified flag when inserting
> data too (as happens for most other operations). (This might be a historical
> artifact of Base having been tested mainly with HSQLDB?)

Yes, there is that horrendous hack that an embedded HSQLDB does a "save
data in the ODB" on commit, which probably led to disabling the save button.
With Firebird, which I understand saves the data on an explicit "save the
ODB" action, the save button should indeed be activated on insert, modify,
delete (any data change).

> That would however mean that in Firebird data is only ever commited to the
> .odb file with a "Save", whereas HSQLDB always has its data stored
> immediately, i.e. there'd be a significant difference between the operation
> of the two dbs in practice.

I though we already had decided they would have this difference, *but*
taking care that autosaves / backup saves have fresh data (not the same
data as the last save), so as to have some level of crash tolerance.
Comment 4 Terrence Enger 2013-10-06 17:45:16 UTC
Indeed, it does come down to "Save" being disabled.  If you do
something else in the file to enable Save, then you can Save; and this
captures added rows.

For comparison, if you Save from the Table Data View while its Save
icon is enabled (e.g., step 4 of the original STR), it does nothing
that I can see.

Terry.
Comment 5 Terrence Enger 2013-10-10 20:22:19 UTC
Changing summary to more closely reflect the cause of the problem.  For the convenience of anybody searching for the symptom that I reported, the original summary was:

    cannot add row to table in embedded Firebird db
Comment 6 robert 2013-10-13 13:55:05 UTC
(In reply to comment #5)
> Changing summary to more closely reflect the cause of the problem.  For the
> convenience of anybody searching for the symptom that I reported, the
> original summary was:
> 
>     cannot add row to table in embedded Firebird db

Have tested it with
Build ID: b1caf176a44b6979d2e0ea47f495a3dacf86e197
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:master, Time: 2013-10-12_06:04:49

Why did you change the summary. I have created a database with only one table. Could input new rows, could press save during input, could move through the rows, could close the table, could reopen the table and see the rows - but when I close the database and reopen it all data were lost. The database loose not some, but all changes.
Comment 7 Terrence Enger 2013-10-13 14:50:05 UTC
(In reply to comment #6)
> Why did you change the summary.
> [snip]
> The database loose not some, but all changes.

If you do an action that the program *does* recognize as changing the
should-be-saved .odb file, then the program enables the Save option
and upon closing the file prompts you if you have not saved your
changes.  If you save your changes, the data changes are saved too.
For example, if you create a query and save it from the query creation
window, the program recognizes a change impacting the .odb.  So, the
program does not always lose all added rows.

Creation of a table (IIRC) is another case where the program does not
realize that the .odb is impacted.  So the program loses some other
changes beside added or changed rows.

It is hard to write all this in one line.

Terry.
Comment 8 Andrzej Hunt 2013-11-08 14:38:41 UTC
So, looking at things, it looks like one way of attacking the problem is simply sprinkling plenty of:
    ::dbaccess::notifyDataSourceModified(m_xParent,sal_True);
wherever we've modified any data in the database within dbaccess.

(See https://gerrit.libreoffice.org/#/c/6620/ for an example which fixes the the problem for inserting data into a single table.)

However this would mark the document as changed even if changing data in an external database -- hence I think it might make more sense to instead notify the document that it has changed from the database driver itself, and/or test that we are working with an embedded db before marking the document as changed? (I'm currently still looking at the different options for doing this.)
Comment 9 Andrzej Hunt 2013-11-08 15:46:19 UTC
> However this would mark the document as changed even if changing data in an
> external database -- hence I think it might make more sense to instead
> notify the document that it has changed from the database driver itself,
> and/or test that we are working with an embedded db before marking the
> document as changed? (I'm currently still looking at the different options
> for doing this.)

This does in fact seem to be the best solution -- patch in https://gerrit.libreoffice.org/#/c/6621/ , however this only fixes things for editing existing tables (and deleting tables), inserting a new table still doesn't result in the save button being enabled.
Comment 10 Commit Notification 2013-11-14 12:04:08 UTC
Andrzej J.R. Hunt committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=acef4b26879f6f8d80c26f6d2ee80667cb6ed388

fdo#70144 Mark database document as modified when data inserted in firebird.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 11 Terrence Enger 2013-11-20 19:55:11 UTC
The problem is fixed in my debug build of master commit 3d7662, fetched 2013-11-19.
Comment 12 Commit Notification 2013-12-04 13:54:05 UTC
Andrzej J.R. Hunt committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=56d4f4b7b97eaf212a3bc6bdb1da767106e7224a

fdo#70144 Don't reset Document's modified flag when inserting new table.



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 13 Commit Notification 2013-12-04 19:51:42 UTC
Andrzej J.R. Hunt committed a patch related to this issue.
It has been pushed to "libreoffice-4-2":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=c6e36556c14612aa1eaf976efc10db6c88e23fb0&h=libreoffice-4-2

fdo#70144 Don't reset Document's modified flag when inserting new table.


It will be available in LibreOffice 4.2.

The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.