After some testing, it appears that the maximu date value is 27/06/9957. Any date value entered which is higher than that triggers automatic conversion to 31/12/9999.

So Base incorrectly refuses to register any date comprised between 28/06/9957 and 30/12/9999.


Alex

I reproduced exactly this behavior with 3.4.4 and on master. (pc Debian x86-32 testing updated today).
Could a dev give some pointers on the code to find the cause ?

Assuming that Base uses tools' class Date,
tools/source/datetime/tdate.cxx line 49 has a magic

#define MAX_DAYS    3636532

that's ~9957 years. Apparently the algorithm underlying in DaysToDate() can't cope with higher values, didn't check, at least all operators adding days to dates calling that method check for MAX_DAYS before doing so.

Though I wouldn't go as far as calling that "a quite significant reduction in the number of years available", really, which star gate does one try to calculate with an office suite..

Just for information, a research for "DaysToDate" in Opengrok shows that it's implemented 3 times :
- scaddins/source/datefunc/datefunc.cxx
- tools/source/datetime/tdate.cxx
- scaddins/source/analysis/analysishelper.cxx
But if DaysToDate function appears called from tools/source/datetime/date.cxx,
MAX_DAYS is also present in connectivity/source/commontools/dbconversion.cpp
which calls implBuildFromRelative

The 3 DaysToDate functions + implBuildFromRelative function seem to want to do the same thing.

(In reply to comment #5)
> 
> Though I wouldn't go as far as calling that "a quite significant reduction in
> the number of years available", really, which star gate does one try to
> calculate with an office suite..

Hmmm, well, maybe if I want to turn my LO into a control interface for my next hyperspace trip, I might end up landing on a Replicator planet and being destroyed by robots ;-)

At the least, a nice little error message that the maximum limit has been reached would be nice ;-)

Alex

Hi Alex

exists your problem still using LO 3.6.0?

Yes.
Tested on Version 3.7.0.0.alpha0+ (Build ID: 3e9f9e5)

Eike: what about removing MAX_DAYS and use:
- sal_Int32 everywhere (instead of long and sal_uInt16)
- sal_IntPtr instead of sal_uIntPtr
- SAL_MAX_INT32 (0x7FFFFFFF -> 4294967295 so far more than 3636532)
?
If ok, I could give a try to propose a patch.

@Julien:
No objection in general, but bear in mind that the Date::nDate value is stored in a decimal coded way to hold YYYYMMDD so the maximum year value in a sal_Int32 would be 214748 (0x7fffffff is 2147483647 not 4294967295 ;-)  As the maximum number of normalized days must be representable by that coded sal_Int32, the resulting MAX_DAYS would be 78383020 (214748*365), do not drop those checks. Instead, verify that they do sane things, i.e. some currently set 9999-12-31 in such a case. Places may also check for year values of 9999 or 9957 or some such, introduce a DATE_MAX_YEAR constant for the 214748 maximum year value and eliminate those hard coded values.

Furthermore not all code may be able to cope with a year value of more than 4 digits, either in input or display formats, field lengths, ... so there may be some follow-up work to be done.

Something more disruptive, but arguably cleaner, would be to stop (let's say in Base / form elements) (ab)using integers to mean dates, and to use com::sun::star::util::Date.

Unless we also "improve" formatters (in ways that other components of LibreOffice, especially Calc, will maybe not like...), I'm afraid this we'd have to go back to integer to format dates... So maybe this route would become quite a "big piece", but nice dream.

(In reply to comment #12)
> Eike: what about removing MAX_DAYS and use:
> - sal_Int32 everywhere (instead of long and sal_uInt16)
> - sal_IntPtr instead of sal_uIntPtr

I very much doubt any date-handling code has any good reason to use either sal_IntPtr or sal_uIntPtr which are "an integer big enough to contain a pointer". The uses of that should very most probably be some sal_(u)Int16/32/64.

Forget my last comment, we can't blindly change all this with sal_Int32 since it's called in several parts.
Moreover, following Lionel's comments, it seems it's not at all an easyhack, hope someone will find a way.

I think changing the internal representation in tools' class Date wouldn't be that hard. To inspect places that might rely on the internal layout of Date::nDate and do calculations on their own and if so fix them to use proper class Date methods, I'd make the following methods private and global make to catch where it breaks:

Date( sal_uInt32 _nDate )
void SetDate( sal_uInt32 nNewDate )
sal_uInt32 GetDate() const


Using css::util::Date would only have advantage over improving tools' Date internals for that it would obsolete the internal back/forth conversion to/from coded integer. Its year field is a short, thus sal_Int16 with a max year value of 32765 which would be less than the 214748 years. (not that I don't doubt any use if such year values anyway ;-)

How you actually represent a date in an application is a different goal, and the spreadsheet applications' way (and not only those) to express a date/time as a double since a null date is yet something different. This is also independent of how you format a date, and the formatter doesn't have to care what the Date class uses internally.

I agree that the sal_uIntPtr should be sal_Int64 instead.

Still present in 

Version: 4.4.7.2
Build ID: f3153a8b245191196a4b6b9abd1d0da16eead600
Locale: fr.UTF-8

Confirming also on 

Version: 5.2.0.0.alpha0+
Build ID: ac91a81b7b47fc9eba24455d34e2168309ea51d1
CPU Threads: 2; OS Version: -; UI Render: default; 
Locale: fr-FR (fr.UTF-8)

Internal tools' class Date representation is now (since 5.2) capable to handle dates between -32768-01-01 and 32767-12-31, including conversion from/to css::util::Date

However, connectivity/source/commontools/dbconversion.cxx still has its own date implementation that probably interferes here. Someone familiar with Base should check what the consequences were if that was eliminated and the common tools Date routines be used. My guess is that various database related things can't cope with years > 9999 because year might be stored using 4 characters, so some extra handling will still be needed there.

(In reply to Eike Rathke from comment #23)

> However, connectivity/source/commontools/dbconversion.cxx still has its own
> date implementation that probably interferes here.

For full clarity, it contains a (re?)implementation of some date utilities, but no date (data structure) implementation per se.

It uses the same MAX_DAYS mentioned in comment 5 in addDays and subDays, but frankly I have no idea why. I just ripped out the MAX_DAYS test and it seems to work fine.

I expect most, if not all, of these utilities would be advantageously replaced by a call to common tools Date routines.

> My guess is that various database
> related things can't cope with years > 9999 because year might be stored
> using 4 characters, so some extra handling will still be needed there.

LibreOffice doesn't know, and doesn't care, how the database engine stores the year. It should just throw the value at the database and if the database it unhappy it will raise an error or truncate the value. Or *maybe* the SDBC driver (that is the code in connectivity/source/drivers/*). Any code outside of that (that is generic to "all of Base" or "all database engines") has no good reason to arbitrarily enforce such a limitation, and I officially give my blessing to anyone that volunteers to rip it out.

The only thing is that if we construct SQL expressions with dates in them (as is done e.g. with calls to DBTypeConversion::toDateString, such as in connectivity/source/commontools/DateConversion.cxx function  DBTypeConversion::toSQLString), then the date MUST be in ISO order YEAR-MONTH-DAY, and year MUST have at least 4 digits to avoid ambiguity. That is "AT LEAST", not necssarily "AT MOST". 


WORK ON C++-LEVEL INFRASTRUCTURE

1) Remove the MAX_DAYS check from addDays and subDays
   (I'm doing that now)
2) Remove the negative value check from addDays and subDays,
   unless it makes sense.
3) Replace utilities in dbconversion.cxx by their tools counterpart;
   implBuildFromRelative and implIsLeapYear are prime targets.
   OTOH toDateString, toTimeString, toDateTimeString probably
   belong dbconversion.cxx
   (any volunteer?)
4) Keep in dbconversion.cxx only those that have no counterpart

WORK ON FORM CONTROLS

There are two kinds of form controls that can show dates:

1) Formatted field: this should be able to handle the same date range as Calc,
   that should be the number of days that fits in C++ "double". If not, that is
   to be fixed. One reason they now don't is the MAX_DAYS check,
   which I'm removing

2) Date Fields: testing shows the maximum value of the "Date max" property is
   01/01/9999. Find out why. Arbitrary limit or actual code limit?
   Raise the limit.

Lionel Elie Mamane committed a patch related to this issue.
It has been pushed to "master":

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

tdf#40575 remove obsolete MAX_DAYS check

It will be available in 5.4.0.

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.

Thanks Lionel :-)

Testing with 

Version: 6.0.0.0.alpha0+
Build ID: 003c2cfaa258e204402a366a105366da74e220a2
CPU threads: 4; OS: Mac OS X 10.12.6; UI render: default; 
Locale: fr-FR (fr_FR.UTF-8); Calc: group

shows than I can successfully enter 9999 as the year, so this part is fixed :-)


For the record, if I try and enter any date beyond that in data entry mode in a table, I get a :

Error inserting the new record

java.lang.IllegalArgumentException

presumably because dates in hsqldb are ISO and the year can only have 4 digits ?

(In reply to Eike Rathke from comment #23)
> Internal tools' class Date representation is now (since 5.2) capable to
> handle dates between -32768-01-01 and 32767-12-31, including conversion
> from/to css::util::Date
> 
> However, connectivity/source/commontools/dbconversion.cxx still has its own
> date implementation that probably interferes here. Someone familiar with
> Base should check what the consequences were if that was eliminated and the
> common tools Date routines be used. My guess is that various database
> related things can't cope with years > 9999 because year might be stored
> using 4 characters, so some extra handling will still be needed there.

@Eike :

Does any of this affect the DateAdd() function in LOBasic ?
Someone has just reported on the German M-L that the help is incorrect when describing how to enter the string for the date (indeed, it does seem wrong, but that is tangential to what is to follow):

Sub example_dateadd
MsgBox DateAdd("m", 1, 31012004) &" - "& DateAdd("M", 1, 31012005)
End Sub

Using the above:

- if use double quote characters around the date string, I get other error messages, such as "unsuported type", or "unsupported operation" depending on which way I type the string separators (dots or slashes);

- if I use as in the above example, i.e. no quotes around what is essentially an integer, I get a MsgBox that displays "31/0/1/32767 - 31/01/32767" which is obviously incorrect.

Is this behaviour related to the changes in date representation support of css::util::Date or does DateAdd() rely on the Database date functions ?

Should I open a separate report for this ?

(In reply to Alex Thurgood from comment #28)


> - if I use as in the above example, i.e. no quotes around what is
> essentially an integer, I get a MsgBox that displays "31/0/1/32767 -
> 31/01/32767" which is obviously incorrect.
> 


I accidentally inserted an extra slash into the first date string above when typing out what is displayed on screen "31/01/32767" - I meant to point out that the year calculation is obviously incorrectly displayed.

(In reply to Alex Thurgood from comment #28)
> Does any of this affect the DateAdd() function in LOBasic ?
Basic date math would be unrelated to Base dates.

> Should I open a separate report for this ?
Yes please.

(In reply to Eike Rathke from comment #30)
> (In reply to Alex Thurgood from comment #28)
> > Does any of this affect the DateAdd() function in LOBasic ?
> Basic date math would be unrelated to Base dates.
> 
> > Should I open a separate report for this ?
> Yes please.

OKay, now known as bug 114011

Alex since year 9999 is dealt now thanks to Eike and Lionel, can we close this one?

Years over 9999 seems to be a philosophic problem. It's the limit for 4-digit year. The limit at 9957 has been solved by Lionel.

If I'm still living in 9000 I promise to solve this problem. ;-)

I will close this one to WORKSFORME.

Hi *

Yes, sorry, this should be resolved fixed due to the commits that have gone in.

(In reply to Alex Thurgood from comment #35)
> Hi *
> 
> Yes, sorry, this should be resolved fixed due to the commits that have gone
> in.

No pb, it's always great to be able to close an old bug! :-)