Bug 51779 - support for SQL datatype INTERVAL
Summary: support for SQL datatype INTERVAL
Status: ASSIGNED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Base (show other bugs)
Version:
(earliest affected)
3.5.4 release
Hardware: All All
: medium enhancement
Assignee: Devansh Varshney
URL:
Whiteboard:
Keywords: difficultyInteresting, easyHack, skillCpp, skillSql
: 152585 (view as bug list)
Depends on: 51523
Blocks: Database-Import
  Show dependency treegraph
 
Reported: 2012-07-06 02:50 UTC by Lionel Elie Mamane
Modified: 2024-03-22 11:25 UTC (History)
12 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 Lionel Elie Mamane 2012-07-06 02:50:47 UTC
+++ This bug was initially created as a clone of Bug #51523 +++

LibreOffice does not support the SQL INTERVAL datatype. That is the datatype to store time *durations*, rather than a point in time (like DATE, TIME, TIMESTAMP).

Our embedded HSQLDB 1.8 also does not support it, but that is no reason not to support it cleanly when connecting to database backends that *do* support it.

Places that need to be changed:

1) offapi/com/sun/star/sdbc/DataType.idl needs a value for INTERVAL

2) connectivity/inc/connectivity/FValue.hxx: class ORowSetValue needs to be
adapted to handle values of that type. Also connectivity
source/commontools/FValue.cxx and undoubtedly many other places, such as
source/commontools/DateConversion.cxx, etc.
Comment 1 Jochen 2012-08-30 07:06:21 UTC Comment hidden (no-value)
Comment 2 Björn Michaelsen 2013-10-04 18:46:40 UTC Comment hidden (no-value)
Comment 3 Alex Thurgood 2015-01-03 17:39:13 UTC Comment hidden (no-value)
Comment 4 Robinson Tryon (qubit) 2015-12-13 13:24:14 UTC Comment hidden (no-value, obsolete)
Comment 5 Robinson Tryon (qubit) 2016-02-18 14:52:14 UTC Comment hidden (no-value, obsolete)
Comment 6 kerem 2016-06-06 17:38:01 UTC
Hi,

I could not add INTERVAL selection to datatypes list. Because I could not find associated .ui file. How can i find? Can you help me?

Thank you.
Comment 7 Lionel Elie Mamane 2016-06-06 17:41:33 UTC
(In reply to kerem from comment #6)

> I could not add INTERVAL selection to datatypes list. Because I could not
> find associated .ui file. How can i find? Can you help me?

Before starting to change UI, it needs to be supported in the backend. See comment 0, "places that need to be changed". Did you do that already?
Comment 8 kerem 2016-06-06 17:52:41 UTC
(In reply to Lionel Elie Mamane from comment #7)

> Before starting to change UI, it needs to be supported in the backend. See
> comment 0, "places that need to be changed". Did you do that already?

Yes, i changed in necessary places. But i am not sure. Because i still haven't find INTERVAL data type. Shall I commit my changes?
Comment 9 Gökay ŞATIR 2020-04-18 11:41:54 UTC
Hello,

Latest versions of HSQLDB seems to support interval data types. Should it be upgraded first?
Comment 10 Buovjaga 2020-04-18 12:14:50 UTC
(In reply to Gökay ŞATIR from comment #9)
> Hello,
> 
> Latest versions of HSQLDB seems to support interval data types. Should it be
> upgraded first?

No, we want to remove embedded HSQLDB completely in favour of Firebird.
Comment 11 Gökay ŞATIR 2020-04-18 13:36:39 UTC
Is there an ongoing discussion about which db system to choose? Like this one:
https://ask.libreoffice.org/en/question/88866/base-firebird-vs-hsqldb-embedded-database/

Or is the decision is made already?

I think SQLite could again be the candidate. Its latest versions support multiple reads at the same time, it handles multiple connections well.

SQLite just needs a wrapper for data types, which can be implemented on top of it.

Also, it has amalgamation, i personally find that feature valuable.
Comment 12 Buovjaga 2020-04-18 14:21:01 UTC
(In reply to Gökay ŞATIR from comment #11)
> Is there an ongoing discussion about which db system to choose? Like this
> one:
> https://ask.libreoffice.org/en/question/88866/base-firebird-vs-hsqldb-
> embedded-database/
> 
> Or is the decision is made already?
> 
> I think SQLite could again be the candidate. Its latest versions support
> multiple reads at the same time, it handles multiple connections well.
> 
> SQLite just needs a wrapper for data types, which can be implemented on top
> of it.
> 
> Also, it has amalgamation, i personally find that feature valuable.

There is no ongoing discussion. The decision was made long ago. Since then, there have been multiple Google Summer of Code projects and also TDF contracts to bring embedded Firebird into a production-ready status.

I highly recommend you to contribute to the meta reports:
https://bugs.documentfoundation.org/showdependencytree.cgi?id=116970&hide_resolved=1
https://bugs.documentfoundation.org/showdependencytree.cgi?id=116968&hide_resolved=1

There are some fundamental data type differences between HSQLDB and Firebird, but the upcoming Firebird 4.0 will solve a couple of these and hopefully a future version will solve more. This would make it easier to migrate existing database files from HSQLDB to Firebird.

https://firebirdsql.org/file/community/conference-2019/1_firebird_on_the_road_from4_to_5.pdf

Hopefully we will see 4.0 during the second quarter of 2020.
Comment 13 Gökay ŞATIR 2020-04-18 14:42:47 UTC
Hello,

Thank you for your comment. I started to read bug reports and will try to debug & contribute to the progress.
Comment 14 Libreoffice user SSO 2022-03-05 07:49:52 UTC
I would like to work on this easyhack!
Comment 15 Buovjaga 2022-03-05 07:58:29 UTC
(In reply to Libreoffice user SSO from comment #14)
> I would like to work on this easyhack!

Feel free to assign to yourself.
Comment 16 Lionel Elie Mamane 2022-03-05 08:52:37 UTC
Welcome. Start with comment 0, and let me know of any specific question you have. Feel free to add me as reviewer to patches on gerrit.

Note that this is "difficultyInteresting", meaning it is classified as one of the more difficult ones, and not an "easy first task" if you have never worked on LibreOffice Base.
Comment 17 Libreoffice user SSO 2022-03-05 15:53:29 UTC
Okay.
Comment 18 Libreoffice user SSO 2022-03-10 06:44:43 UTC
(In reply to Lionel Elie Mamane from comment #16)
> Welcome. Start with comment 0, and let me know of any specific question you
> have. Feel free to add me as reviewer to patches on gerrit.
> 
> Note that this is "difficultyInteresting", meaning it is classified as one
> of the more difficult ones, and not an "easy first task" if you have never
> worked on LibreOffice Base.

Hi! I am having trouble finding the files mentioned in the comment 0 listed under "Places that need to be changed: 2)" Can you help me in figuring out the places that needs changes with respect to the current code base ?
Comment 19 Buovjaga 2022-03-10 06:57:41 UTC
(In reply to Libreoffice user SSO from comment #18)
> (In reply to Lionel Elie Mamane from comment #16)
> > Welcome. Start with comment 0, and let me know of any specific question you
> > have. Feel free to add me as reviewer to patches on gerrit.
> > 
> > Note that this is "difficultyInteresting", meaning it is classified as one
> > of the more difficult ones, and not an "easy first task" if you have never
> > worked on LibreOffice Base.
> 
> Hi! I am having trouble finding the files mentioned in the comment 0 listed
> under "Places that need to be changed: 2)" Can you help me in figuring out
> the places that needs changes with respect to the current code base ?

You did not specify which files you were having trouble with, but I checked and some had incorrect (changed over time?) paths. Keep in mind that you can always use the find command, like

find . -name 'FValue.hxx'

Correct paths for the mentioned files:

include/connectivity/FValue.hxx
connectivity/source/commontools/FValue.cxx
connectivity/source/commontools/DateConversion.cxx
Comment 20 Lionel Elie Mamane 2022-03-13 19:39:22 UTC
Pragat, your email provider rejects my answer to your email as suspected spam... Here's your answer:

offapi/com/sun/star/sdbc/DataType.idl just defines constants to refer to a type, that the rest of the code uses to refer to the datatype. These constants are then used in the C++ code to refer to that type, usually with a switch() structure. It is up to the each code snippet making use of these constants to actually make sense of it, there is no central place that does the mapping. Each place where it is used has to "understand" that DATE is a date and how to treat a date, that NUMERIC is a numeric and how to treat a numeric, etc.

See for example
connectivity/source$ emacs commontools/FValue.cxx

Just pick an unused constant, that is unused both in Java's JDBC
https://docs.oracle.com/javase/8/docs/api/java/sql/JDBCType.html
and in LibreOffice's current DataType.idl
Comment 21 Libreoffice user SSO 2022-03-14 04:51:34 UTC Comment hidden (noise)
Comment 22 Buovjaga 2022-03-14 06:22:41 UTC Comment hidden (noise)
Comment 23 Libreoffice user SSO 2022-03-14 08:24:38 UTC Comment hidden (noise)
Comment 24 Stéphane Guillou (stragu) 2022-12-18 22:51:01 UTC
*** Bug 152585 has been marked as a duplicate of this bug. ***
Comment 25 Alexander 2022-12-24 15:54:15 UTC
Hello, is there any progress on this issue ?

I would have suggested to help you, but I am only good with higher level languages like JS, Java and so on :(

Best Regards,
Alex
Comment 26 Buovjaga 2022-12-25 07:51:27 UTC
Seems like Pragat is not active, so let's reset the assignee.
Comment 27 potatochick2020 2023-03-18 14:51:46 UTC
Is this bug still active? Could I assign this to myself?
Comment 28 Buovjaga 2023-03-18 15:01:38 UTC
(In reply to potatochick2020 from comment #27)
> Is this bug still active? Could I assign this to myself?

Yes
Comment 29 potatochick2020 2023-03-18 18:42:11 UTC
https://www.postgresqltutorial.com/postgresql-tutorial/postgresql-interval/ 
An interval value requires 16 bytes storage

Postgres support interval data type, is that possible to start by trying to follow comment 0, and test it out by connect to an existing postgres database?

Will this be in a good starting point?
Comment 30 Lionel Elie Mamane 2023-03-19 10:47:00 UTC
(In reply to potatochick2020 from comment #29)
> https://www.postgresqltutorial.com/postgresql-tutorial/postgresql-interval/ 
> An interval value requires 16 bytes storage
> 
> Postgres support interval data type, is that possible to start by trying to
> follow comment 0, and test it out by connect to an existing postgres
> database?
> 
> Will this be in a good starting point?

Yes, good plan. I was inspired by trying to use interval datatypes from a PostgreSQL database in LibreOffice to file this bug, actually.
Comment 31 Devansh Varshney 2024-02-16 15:41:22 UTC
I found a file basic/source/runtime/methods1.cxx -
which has an enum and some definitions -

Is this related? As I also created some new files related to Interval

enum Interval
{
    INTERVAL_YYYY,
    INTERVAL_Q,
    INTERVAL_M,
    INTERVAL_Y,
    INTERVAL_D,
    INTERVAL_W,
    INTERVAL_WW,
    INTERVAL_H,
    INTERVAL_N,
    INTERVAL_S
};

struct IntervalInfo
{
    Interval    meInterval;
    char const * mStringCode;
    double      mdValue;
    bool        mbSimple;
};

}

static IntervalInfo const * getIntervalInfo( const OUString& rStringCode )
{
    static IntervalInfo const aIntervalTable[] =
    {
        { INTERVAL_YYYY, "yyyy", 0.0,           false }, // Year
        { INTERVAL_Q,    "q",    0.0,           false }, // Quarter
        { INTERVAL_M,    "m",    0.0,           false }, // Month
        { INTERVAL_Y,    "y",    1.0,           true  }, // Day of year
        { INTERVAL_D,    "d",    1.0,           true  }, // Day
        { INTERVAL_W,    "w",    1.0,           true  }, // Weekday
        { INTERVAL_WW,   "ww",   7.0,           true  }, // Week
        { INTERVAL_H,    "h",    1.0 /    24.0, true  }, // Hour
        { INTERVAL_N,    "n",    1.0 /  1440.0, true  }, // Minute
        { INTERVAL_S,    "s",    1.0 / 86400.0, true  }  // Second
    };
    auto const pred = [&rStringCode](const IntervalInfo &aInterval) {
            return rStringCode.equalsIgnoreAsciiCaseAscii(aInterval.mStringCode);
    };

    auto intervalIter = std::find_if(std::begin(aIntervalTable), std::end(aIntervalTable), pred);
    if(intervalIter != std::end(aIntervalTable)) {
            return intervalIter;
    }
    return nullptr;
}
Comment 32 Lionel Elie Mamane 2024-02-19 11:56:14 UTC
(In reply to Devansh Varshney from comment #31)
> I found a file basic/source/runtime/methods1.cxx -
> which has an enum and some definitions -
> 
> Is this related?

This looks like an implementation of time interval in Basic; similar concept, but I don't think it will be directly useful for this enhancement.
Comment 33 Devansh Varshney 2024-03-19 12:27:35 UTC
I added the entry for the SQL INTERVAL in the DataType.idl file as mentioned and also in the offapi/type_reference/offapi.idl after asking from Buovjaga.

Here, is the related PR to it - https://gerrit.libreoffice.org/c/core/+/165012



I am kinda getting in one trouble when I am trying to build the project initially 
it wasn't generating the required .hdl and the .hpp files to add the support in the respective FValue files.

After-awhile I run the autogen again and make, then it triggered the related files(offapi, hsqldb, firebird, sql, etc.) but my terminal is crashing almost at the end of build everytime I do this. (perhaps less RAM?)

And to try things again I did created a new local branch and cleared the ccache and something happened.
Here, are the screenshot of them - https://postimg.cc/gallery/ygzpx8J



Moreover, there are couple of places where some definition related to the INTERVAL -

 line 84: connectivity/source/drivers/odbc/OTools.cxx
 line 428, 1355: external/unixODBC/inc/odbc/sqlext.h
Comment 34 Michael Weghorn 2024-03-21 08:22:27 UTC
(In reply to Devansh Varshney from comment #33)
> I added the entry for the SQL INTERVAL in the DataType.idl file as mentioned
> and also in the offapi/type_reference/offapi.idl after asking from Buovjaga.
> 
> Here, is the related PR to it -
> https://gerrit.libreoffice.org/c/core/+/165012

This is currently still in "Work in progress" state. Once you think this is ready for review (likely after implementing the actual handling, see comment 0 for code pointers), I'd suggest to mark it as such.

> I am kinda getting in one trouble when I am trying to build the project
> initially 
> it wasn't generating the required .hdl and the .hpp files to add the support
> in the respective FValue files.
> 
> After-awhile I run the autogen again and make, then it triggered the related
> files(offapi, hsqldb, firebird, sql, etc.) but my terminal is crashing
> almost at the end of build everytime I do this. (perhaps less RAM?)

You could try with environment variable `PARALLELISM` set to reduce the number of processes run in parallel, e.g. `PARALLELISM=1 make` (probably something larger than 1 should work, you can try that)
Comment 35 Devansh Varshney 2024-03-21 17:25:06 UTC
(In reply to Michael Weghorn from comment #34)

> You could try with environment variable `PARALLELISM` set to reduce the
> number of processes run in parallel, e.g. `PARALLELISM=1 make` (probably
> something larger than 1 should work, you can try that)

It got resolved thanks to Ilmari. I had set the PARALLELISM=6 earlier then I set it to 4 and it worked. But, still I can't find the generated .hpp and .hdl files where for example Date.hpp and Date.hdl files are.
Comment 36 Lionel Elie Mamane 2024-03-21 17:34:20 UTC
(In reply to Devansh Varshney from comment #35)

> It got resolved thanks to Ilmari. I had set the PARALLELISM=6 earlier then I
> set it to 4 and it worked. But, still I can't find the generated .hpp and
> .hdl files where for example Date.hpp and Date.hdl files are.

Date.hdl and Date.hpp are generated from offapi/com/sun/star/util/Date.idl and they are in workdir/UnoApiHeadersTarget/offapi/normal/com/sun/star/util/Date.h{pp,dl}
Comment 37 Devansh Varshney 2024-03-22 06:42:22 UTC
Is this how the Interval.idl file looks like?

module com { module sun { module star { module sdbc {

/** represents an SQL INTERVAL value.
 */
published struct SQLINTERVAL
{
    /** indicates the interval class (YEAR_MONTH or DAY_TIME).
     */
    long IntervalClass;

    /** indicates the leading field (YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, or FRACTION).
     */
    long LeadingField;

    /** indicates the trailing field (YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, or FRACTION).
     */
    long TrailingField;

    /** contains the number of years (0 to 9999).
     */
    long Years;

    /** contains the number of months (0 to 11).
     */
    long Months;

    /** contains the number of days (0 to 999999999).
     */
    long Days;

    /** contains the number of hours (0 to 23).
     */
    long Hours;

    /** contains the number of minutes (0 to 59).
     */
    long Minutes;

    /** contains the number of seconds (0 to 59).
     */
    long Seconds;

    /** contains the number of nanoseconds (0 to 999999999).
     */
    long NanoSeconds;

    /** indicates the leading field precision (1 to 9 for YEAR, MONTH, DAY, HOUR, MINUTE, SECOND; 1 to 5 for FRACTION).
     */
    long LeadingFieldPrecision;

    /** indicates the fraction scale (0 to 5).
     */
    long FractionScale;

    /** indicates the interval sign (1 for positive, -1 for negative).
     */
    long Sign;
};

}; }; }; };

And thank you Lionel for pointing the Date.idl directory I did earlier made entries in the offapi.idl and Datatype.idl;
Comment 38 Devansh Varshney 2024-03-22 06:49:51 UTC
(In reply to Devansh Varshney from comment #37)
> 
>     /** contains the number of years (0 to 9999).
>      */
>     long Years;
> 
>     /** contains the number of months (0 to 11).
>      */
>     long Months;
> 
>     /** contains the number of days (0 to 999999999).
>      */
>     long Days;
> 

/** contains the number of years.
*/

/** is the month of year (1-12 or 0 for a void date).

*/

/** is the day of month (1-31 or 0 for a void date).

*/
Comment 39 Devansh Varshney 2024-03-22 07:31:07 UTC
This is the corrected -

module com { module sun { module star { module sdbc {

/** represents an SQL INTERVAL value.
 */
published struct SQLINTERVAL
{
    /** indicates the interval class (YEAR_MONTH or DAY_TIME).
     */
    unsigned short IntervalClass;

    /** indicates the leading field (YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, or FRACTION).
     */
    unsigned short LeadingField;

    /** indicates the trailing field (YEAR, MONTH, DAY, HOUR, MINUTE, SECOND, or FRACTION).
     */
    unsigned short TrailingField;

    /** contains the number of years
     */
    long Years;

    /** contains month of year (1-12 or 0 for a void date).
     */
    unsigned short Months;

    /** contains day of month (1-31 or 0 for a void date).
     */
    unsigned short Days;

    /** contains the number of hours (0 to 23).
     */
    unsigned short Hours;

    /** contains the number of minutes (0 to 59).
     */
    unsigned short Minutes;

    /** contains the number of seconds (0 to 59).
     */
    unsigned short Seconds;

    /** contains the number of nanoseconds (0 to 999999999).
     */
    unsigned long NanoSeconds;

    /** indicates the leading field precision (1 to 9 for YEAR, MONTH, DAY, HOUR, MINUTE, SECOND; 1 to 5 for FRACTION).
     */
    unsigned short LeadingFieldPrecision;

    /** indicates the fraction scale (0 to 5).
     */
    unsigned short FractionScale;

    /** indicates the interval sign (1 for positive, -1 for negative).
     */
    short Sign;
};

}; }; }; };



https://www.postgresqltutorial.com/postgresql-tutorial/postgresql-interval/
An interval value requires 16 bytes of storage that can store a period with the allowed range from -178,000,000 years to 178,000,000 years.


I want to ask -
The Years, Months, Days, Hours, Minutes, Seconds, and NanoSeconds fields should be kept as long to allow for both positive and negative values? The range of these fields should be sufficient to cover the allowed range from -178,000,000 years to 178,000,000 years
https://firebirdsql.org/refdocs/langrefupd21-intfunc-dateadd.html
Comment 40 Lionel Elie Mamane 2024-03-22 08:57:18 UTC
(In reply to Devansh Varshney from comment #39)
> published struct SQLINTERVAL
> {
>     /** indicates the interval class (YEAR_MONTH or DAY_TIME).
>      */
>     unsigned short IntervalClass;
> 
>     /** indicates the leading field (YEAR, MONTH, DAY, HOUR, MINUTE, SECOND,
> or FRACTION).
>      */
>     unsigned short LeadingField;
> 
>     /** indicates the trailing field (YEAR, MONTH, DAY, HOUR, MINUTE,
> SECOND, or FRACTION).
>      */
>     unsigned short TrailingField;

I may be missing something, but what is the meaning of these and why are they useful? Is an interval/duration not just a number of years, months, days, hours, minutes, seconds and nanoseconds?

While the datatype at the storage (database level) may specify a precision/granularity (e.g. store only years and months or store only hours, minutes and seconds), why do we need to carry this over in the values of our API? E.g. when we get from the database a value from a field NUMERIC(9,2), we just give out a float, we don't carry over that it was restricted to two digits after the decimal point. Why are intervals different and should carry over that information?
Comment 41 Devansh Varshney 2024-03-22 10:39:44 UTC
(In reply to Lionel Elie Mamane from comment #40)

> 
> I may be missing something, but what is the meaning of these and why are
> they useful? Is an interval/duration not just a number of years, months,
> days, hours, minutes, seconds and nanoseconds?

I just added these to get a confirmation after going through some other blogs -
https://www.ibm.com/docs/en/informix-servers/12.10?topic=types-interval-data-type
https://oracle-base.com/articles/misc/oracle-dates-timestamps-and-intervals#interval




> While the datatype at the storage (database level) may specify a
> precision/granularity (e.g. store only years and months or store only hours,
> minutes and seconds), why do we need to carry this over in the values of our
> API? E.g. when we get from the database a value from a field NUMERIC(9,2),
> we just give out a float, we don't carry over that it was restricted to two
> digits after the decimal point. Why are intervals different and should carry
> over that information?

I thought while these fields may not be directly used by all databases, they can be useful when working with databases that support interval types with varying precision and formats. I thought for the flexibility for future enhancements or scenarios where this information may be needed in future can be useful for formatting, calculations, or data validation purposes.


I am removing these from the struct and just one question regarding the 

long Years; 

should I go with long? as in other idl files it's short.
Comment 42 Lionel Elie Mamane 2024-03-22 11:25:22 UTC
(In reply to Devansh Varshney from comment #41)
> I am removing these from the struct and just one question regarding the 
> 
> long Years; 
> 
> should I go with long? as in other idl files it's short.

long looks good to me. short would have a lower range than what PostgreSQL allows.