Bug 138409 - FIREBIRD: decimal fields in table are falsifying their default values
Summary: FIREBIRD: decimal fields in table are falsifying their default values
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
7.1.0.0.alpha1+
Hardware: x86-64 (AMD64) Linux (All)
: medium normal
Assignee: Caolán McNamara
URL:
Whiteboard: target:7.2.0 target:7.1.0
Keywords:
Depends on:
Blocks: Database-Firebird-Default
  Show dependency treegraph
 
Reported: 2020-11-22 14:07 UTC by Richard Demattio
Modified: 2021-01-27 15:31 UTC (History)
4 users (show)

See Also:
Crash report or crash signature:


Attachments
DB to show the bug (13.02 KB, application/vnd.oasis.opendocument.database)
2020-11-22 14:10 UTC, Richard Demattio
Details
HSQLDB embedded Test file ok (created with en-US locale) (3.33 KB, application/vnd.oasis.opendocument.database)
2020-12-06 16:32 UTC, Julien Nabet
Details
Firebird embedded Test file ok (created with en-US locale) (2.82 KB, application/vnd.oasis.opendocument.database)
2020-12-06 16:51 UTC, Julien Nabet
Details
HSQLDB showing different defaults - but saves the same ... (3.45 KB, application/vnd.oasis.opendocument.database)
2020-12-06 17:12 UTC, Robert Großkopf
Details
Firebird showing different defaults - saves decimalseperator "," devided by 100 (5.33 KB, application/vnd.oasis.opendocument.database)
2020-12-06 19:23 UTC, Robert Großkopf
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Demattio 2020-11-22 14:07:36 UTC
Description:
probably important: my "options / language settings"
+ user interface:     default - english US
+ locale setting:     German (Austria)
+ decimal separator key: same as locale setting (,) {comma}

initial status: default values for fields are defined with two decimals

the values are shown correctly in edit mode
in operations mode the part behind the comma looks truncated 
AND
when I create a record directly in the table, the default value is 
 - in my test database - divided by 100

AND 
When I start creating the record directly in the table I get on the terminal:
** (soffice:7008): WARNING **: 14:58:35.046: Focused object has invalid index in parent



Steps to Reproduce:
1.enter a new record in the database, attached
2.
3.

Actual Results:
wrong values derived from default values in decimal fields

Expected Results:
unchanged default values


Reproducible: Always


User Profile Reset: No


OpenGL enabled: Yes

Additional Info:
Version: 7.1.0.0.alpha1+
Build ID: 4486ab59348ddbfa4b050195477c2842c0a7de0a
CPU threads: 4; OS: Linux 5.8; UI render: default; VCL: gtk3
Locale: de-AT (en_US.UTF-8); UI: en-US
TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:master, Time: 2020-11-21_13:22:04
Calc: threaded
Comment 1 Richard Demattio 2020-11-22 14:10:18 UTC
Created attachment 167474 [details]
DB to show the bug

by the way: same behaviour in 7.0.3.1
Comment 2 Robert Großkopf 2020-11-22 18:35:28 UTC
Have tested this one. Could confirm the buggy behavior for the internal Firebird database. Defaults for decimal fields will be divided by 100 when saving the row.
Comment 3 Julien Nabet 2020-12-06 15:41:47 UTC
Decimal seperator issue:
when it doesn't work
Thread 1 "soffice.bin" hit Breakpoint 2, connectivity::firebird::OPreparedStatement::setObjectWithInfo (this=0x54623f0, parameterIndex=9, x=uno::Any("string": "32,22"), sqlType=3, scale=2)
    at connectivity/source/drivers/firebird/PreparedStatement.cxx:764


when it works:
Thread 1 "soffice.bin" hit Breakpoint 2, connectivity::firebird::OPreparedStatement::setObjectWithInfo (this=0x45c3e30, parameterIndex=1, x=uno::Any("string": "123.25"), sqlType=3, scale=2)
    at connectivity/source/drivers/firebird/PreparedStatement.cxx:764


It could be interesting to know if it's Firebird only.
I mean, I don't see in the setObjectWithInfo methods of other drivers (Mysql, Postgresql,...) any specific treatment to deal with separator.
Comment 4 Robert Großkopf 2020-12-06 16:16:49 UTC
(In reply to Julien Nabet from comment #3)> 
> 
> It could be interesting to know if it's Firebird only.
> I mean, I don't see in the setObjectWithInfo methods of other drivers
> (Mysql, Postgresql,...) any specific treatment to deal with separator.

Hi Julien,

have tested it with internal HSQLDB. Set a default for a decimal field to "3,75" and formatted it for showing the 2 decimal places. When opening the table I see "3,00" instead. And this is the value which will be saved in HSQLDB.
If I set 3.75 I have to choose English as language for the decimal separator in the table editor to format it with 2 decimal places. Then I open the table for input data and it will show 3.75. Now I changed this German decimal separator and it works: 3,75.

So it is a different behavior, but it would only work right with the point as decimal separator.
Tested with LO 7.0.4.1 on OpenSUSE 15.1 64bit rpm Linux.
Comment 5 Julien Nabet 2020-12-06 16:27:22 UTC
Thank you Robert for the quick feedback, could you attach the hsqldb embedded file when decimal is well managed?
Comment 6 Julien Nabet 2020-12-06 16:32:03 UTC
Created attachment 167876 [details]
HSQLDB embedded Test file ok (created with en-US locale)

Robert: don't bother, once you change locale setting, it's very straightforward.
Comment 7 Julien Nabet 2020-12-06 16:51:08 UTC
Created attachment 167877 [details]
Firebird embedded Test file ok (created with en-US locale)

Here's a Firebird example which works.
Notice that with a locale which has decimal separator as "," the default value when editing table doesn't appear but the by-default insert works.

With English-US locale, everything is ok.

Are we agree to tell it's not Firebird specific?
Comment 8 Julien Nabet 2020-12-06 17:11:28 UTC
With the last test file attached (Firebird embedded test file ok (created with en-US locale), I made this other test:
- change to French locale (so decimal separator is ",")
- open this last file (editing table field shows that it can't display by default value as I previously indicated)
- create a new table with:
- "Decimal places":2
- "Default value":12,22
- save

As expected, inserting a new row doesn't work, it display "12,00" then save "0,12" but when editing table field, everything seems ok.

Lionel: I think the only place to change is field edition, for both when reading and when writing.
I'm quite sure that internally, the decimal separator should/must be stored with a "." (since most programming languages are in English).
To make decimal separator "," work, we need an extra display treatment layer. This is done when inserting records in table (the first one displays "," whereas it's been created with en-US locale) but not when editing a field of a table.
What do you think?
Comment 9 Robert Großkopf 2020-12-06 17:12:07 UTC
Created attachment 167878 [details]
HSQLDB showing different defaults - but saves the same ...

The first decimal field is set to default 3,75 in the table GUI. It will be shown as 3,00 outside the GUI.
The second decimal field is set to default 3.75 in the table GUI. It will be shown outside as 3.75, but could be changed in the table to 3,75.

Both fields will save the GUI default value 3,75!
Comment 10 Julien Nabet 2020-12-06 17:25:38 UTC
(In reply to Robert Großkopf from comment #9)
> Created attachment 167878 [details]
> HSQLDB showing different defaults - but saves the same ...
> 
> The first decimal field is set to default 3,75 in the table GUI. It will be
> shown as 3,00 outside the GUI.
> The second decimal field is set to default 3.75 in the table GUI. It will be
> shown outside as 3.75, but could be changed in the table to 3,75.
> 
> Both fields will save the GUI default value 3,75!

It seems it gets even more complicated :-( Just for curiosity, did you create this file by using only 1 pass so with 1 locale only?
If yes which one?
If no, did you begin with en-US locale then German local or the reverse?
Comment 11 Robert Großkopf 2020-12-06 19:20:27 UTC
(In reply to Julien Nabet from comment #10)
> 
> It seems it gets even more complicated :-( Just for curiosity, did you
> create this file by using only 1 pass so with 1 locale only?
> If yes which one?
> If no, did you begin with en-US locale then German local or the reverse?

I have created it in German locale. Switched only the language in the formatting dialog.

If I try the same with Firebird it will show the same values for GUI-default of the table (3,00 and 3,75), but it will save it as 0,03 and 3,75.

Let us first have a look to what is going wrong with 1/100 in Firebird. This should be solved here. After this is solved the wrong shown values for default with decimal separator "," would be the second bug.
Comment 12 Robert Großkopf 2020-12-06 19:23:48 UTC
Created attachment 167880 [details]
Firebird showing different defaults - saves decimalseperator "," devided by 100
Comment 13 Julien Nabet 2020-12-06 19:26:14 UTC
I may be wrong but I think decimal separator issue and divide by 100 are related.
I mean when you create an odb from scratch with en-US locale (or any locale which uses "." for decimal separator) and stay on it, there are no pbs at all (or did I miss it?).
Comment 14 Robert Großkopf 2020-12-06 19:36:55 UTC
(In reply to Julien Nabet from comment #13)
> I may be wrong but I think decimal separator issue and divide by 100 are
> related.
> I mean when you create an odb from scratch with en-US locale (or any locale
> which uses "." for decimal separator) and stay on it, there are no pbs at
> all (or did I miss it?).

You are right. No problems with a locale, which works with "." as decimal separator.

I think this bug hasn't been detected, because the aren't much people (with locale using separator ",") using this GUI-default for decimal values.
Comment 15 Julien Nabet 2020-12-15 21:24:21 UTC
Caolán: I think it's not a Base specific pb but rather a decimal separator pb. Even if it's not gtk3 specific here, noticing https://cgit.freedesktop.org/libreoffice/core/commit/?id=b163f838c79118809d880c76f943fdd2fc0288ed, thought you might have some idea how to fix this one or have some opinion here?

(if needed, to create an embedded Firebird, you need to enable experimental features).
Comment 16 Caolán McNamara 2020-12-17 14:18:27 UTC
I'm sure its obvious what the bug is and how to reproduce it, but I really need very basic steps
Comment 17 Julien Nabet 2020-12-17 18:10:19 UTC
(In reply to Caolán McNamara from comment #16)
> I'm sure its obvious what the bug is and how to reproduce it, but I really
> need very basic steps

Steps:
1) Retrieve https://bugs.documentfoundation.org/attachment.cgi?id=167474 (initial attachment of bugtracker)

2) Choose a locale setting in LO which has "," for decimal separator
Tools/Options/Language Settings/Languages
Locale Setting, you can try "French (France)"
+ restart LO

3) In main Base panel, click "Tables" icon so LO shows available tables
and double click on the only table "testDefaults"

4) on third line, click "Changethis" cell
notice b_decimal cell of 3rd line displays "22,00"
Replace the by-default content of "Changethis" cell with any value and type "Enter" twice to generate a fourth line.

The cell which displayed "22,00" now displays "0.22"

Hope it'll help to reproduce this, don't hesitate to ask if I miss a step! :-)
Comment 18 Caolán McNamara 2021-01-21 20:19:24 UTC
what I know so far:

dbaccess/source/core/api/KeySet.cxx
  OKeySet::executeInsert is what's called when the newly edited row is added to the table
  impl_convertValue_throw searches for a "." so at this point the values should definitely be in a .-using format, if an explicit e.g. 11,11 is typed 11.11 is seen here.
If the default of 22,22 is used then 22,22 appears here. It looks to me that LibreOffice is using OUString::toIntX and will get "22" when it parses that. And I feel that firebird is given the raw "22,22" and it's firebird that parses than in a value 0.22, I certainly see in
connectivity/source/commontools/FValue.cxx
  ORowSetValue::impl_fill that we get a literal string of 0.22 back when we read the row.

its also the fact that on unzipping the .odb that the xml contains
db:default-value="22,22" so the 'bad' default value is in the source document.

So I think the problem is at the initial setting of the default value via the UI letting through a literal "22,22" as a default value instead of turning it into 22.22. I presume this value is set by using the "Default value" field of the "Field Properties" panel in the design view.

dbaccess/source/ui/control/FieldDescControl.cxx
  OFieldDescControl::SaveData just takes the string as
sDefault = m_xDefault->get_text() here

interestingly if you enter e.g. 70 click on another row and return it is formatted as 70,00. When we initially set the controlvalue the raw "70" is set as the default string but OFieldDescControl::DisplayData uses OFieldDescControl::getControlDefault to format 70 to 70,00 and sets that as the text in the widget.

my guess is that at that sDefault = m_xDefault->get_text() location we should do something similar to the reverse of getControlDefault to turn sDefault in a .-using number string for appropriate types of fields
Comment 19 Caolán McNamara 2021-01-21 20:56:53 UTC
wip of https://gerrit.libreoffice.org/c/core/+/109772 along the lines of that theory. You have to change the defaults to new defaults via the UI for the change to take effect.
Comment 20 Commit Notification 2021-01-22 11:01:22 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/90f2a582a2d5e435012ec38e50022f41b04ae882

tdf#138409 numerical ControlFormat strings shouldn't be localized

It will be available in 7.2.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 Caolán McNamara 2021-01-22 11:04:01 UTC
I think that's the fundamental problem, fixed in master and backport to 7-1 in gerrit. I can't think of a safe way to "repair" existing files, but new ones, or manually modified ones should work IIUC.
Comment 22 Commit Notification 2021-01-22 11:19:41 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-1":

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

tdf#138409 numerical ControlFormat strings shouldn't be localized

It will be available in 7.1.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 23 Commit Notification 2021-01-27 14:44:40 UTC
Caolán McNamara committed a patch related to this issue.
It has been pushed to "libreoffice-7-1-0":

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

tdf#138409 numerical ControlFormat strings shouldn't be localized

It will be available in 7.1.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.