Reimplement datediff and dateadd in c to improve performance #1998

Jakeowen1 · 2023-11-08T19:55:08Z

Description

This change reimplements the datediff, datediff_big, and dateadd functions in C to improve performance by 65% compared to the original implementation.

Issues Resolved

Task: Babel-4496

Test Scenarios Covered

Use case based -

1> select datediff(hour, cast("2023-01-01 01:01:20.99" as datetime), cast("2024-01-01 01:01:20.99" as datetime))
2> go
datediff   
-----------
       8760

(1 rows affected)

Boundary conditions -

1> select datediff(week, cast("2023-01-01 01:01:20.99" as datetime), cast("2023-01-05 01:01:20.99" as datetime))
2> go
datediff   
-----------
          1

(1 rows affected)

Arbitrary inputs -

1> select dateadd(dayofyear, 5, cast("text" as datetimeoffset));
2> go
Msg 33557097, Level 16, State 1, Server BABELFISH, Line 1
invalid input syntax for type timestamp with time zone: "text"

1> select dateadd(dayofyear, 5, null)
2> go
dateadd                
-----------------------
                   NULL

(1 rows affected)

1> select datediff(y, NULL, cast("1900-01-02" as datetime))
2> go
datediff   
-----------
       NULL

(1 rows affected)

Negative test cases -

1> select datediff(nanosecond, cast("2023-02-15" as datetime), cast("1950-02-20" as datetime))
2> go
Msg 33557097, Level 16, State 1, Server BABELFISH, Line 1
The datediff function resulted in an overflow. The number of dateparts separating two date/time instances is too large. Try to use datediff with a less precise datepart
1>

1> select dateadd(day, 2, cast("01:01:21" as time));
2> go
Msg 33557097, Level 16, State 1, Server BABELFISH, Line 1
the datepart "day" is not supported by function dateadd for datatype time

Minor version upgrade tests -
Major version upgrade tests -
Performance tests -

Join on two tables with 60,000 rows each

Query 1:

select d.D
    from dates d
    join more_dates md
        on d.Id = md.Id 
    where datediff(day, d.D, md.D) = 0
GO

Performance with commit:
~~START~~
text
Query Text: select d.D
    from dates d
    join more_dates md
        on d.Id = md.Id 
    where datediff(day, d.D, md.D) = 0
Hash Join  (cost=1816.75..2978.79 rows=332 width=8) (actual time=91.801..677.766 rows=60001 loops=1)
  Hash Cond: (d.id = md.id)
  Join Filter: (datediff('day'::text COLLATE "default", d.d, md.d) = 0)
  ->  Seq Scan on dates d  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.046..56.757 rows=60001 loops=1)
  ->  Hash  (cost=988.00..988.00 rows=66300 width=12) (actual time=89.527..89.529 rows=60001 loops=1)
        Buckets: 131072  Batches: 1  Memory Usage: 3837kB
        ->  Seq Scan on more_dates md  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.040..43.681 rows=60001 loops=1)
Planning Time: 0.020 ms
Execution Time: 747.474 ms
~~END~~

Performance without commit:
~~START~~
text
Query Text: select d.D
    from dates d
    join more_dates md
        on d.Id = md.Id 
    where datediff(day, d.D, md.D) = 0
Hash Join  (cost=1816.75..2978.79 rows=332 width=8) (actual time=94.734..2920.237 rows=60001 loops=1)
  Hash Cond: (d.id = md.id)
  Join Filter: (datediff('day'::text COLLATE "default", d.d, md.d) = 0)
  ->  Seq Scan on dates d  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.049..59.248 rows=60001 loops=1)
  ->  Hash  (cost=988.00..988.00 rows=66300 width=12) (actual time=89.456..89.458 rows=60001 loops=1)
        Buckets: 131072  Batches: 1  Memory Usage: 3837kB
        ->  Seq Scan on more_dates md  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.042..43.743 rows=60001 loops=1)
Planning Time: 0.014 ms
Execution Time: 3002.463 ms
~~END~~


Query 2: 
select dateadd(day, 1, d.D)
    from dates d
    join more_dates md
        on d.Id = md.Id 
    where datediff(day, d.D, md.D) = 0
GO

Performance with commit: 
~~START~~
text
Query Text: select dateadd(day, 1, d.D)
    from dates d
    join more_dates md
        on d.Id = md.Id 
    where datediff(day, d.D, md.D) = 0
Hash Join  (cost=1816.75..3061.79 rows=332 width=8) (actual time=80.382..1347.963 rows=60001 loops=1)
  Hash Cond: (d.id = md.id)
  Join Filter: (datediff('day'::text COLLATE "default", d.d, md.d) = 0)
  ->  Seq Scan on dates d  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.015..42.137 rows=60001 loops=1)
  ->  Hash  (cost=988.00..988.00 rows=66300 width=12) (actual time=77.801..77.803 rows=60001 loops=1)
        Buckets: 131072  Batches: 1  Memory Usage: 3837kB
        ->  Seq Scan on more_dates md  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.011..33.449 rows=60001 loops=1)
Planning Time: 0.013 ms
Execution Time: 1411.302 ms
~~END~~

Performance without commit
~~START~~
text
Query Text: select dateadd(day, 1, d.D)
    from dates d
    join more_dates md
        on d.Id = md.Id 
    where datediff(day, d.D, md.D) = 0
Hash Join  (cost=1816.75..3061.79 rows=332 width=8) (actual time=81.898..4002.259 rows=60001 loops=1)
  Hash Cond: (d.id = md.id)
  Join Filter: (datediff('day'::text COLLATE "default", d.d, md.d) = 0)
  ->  Seq Scan on dates d  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.016..48.974 rows=60001 loops=1)
  ->  Hash  (cost=988.00..988.00 rows=66300 width=12) (actual time=78.335..78.337 rows=60001 loops=1)
        Buckets: 131072  Batches: 1  Memory Usage: 3837kB
        ->  Seq Scan on more_dates md  (cost=0.00..988.00 rows=66300 width=12) (actual time=0.010..33.678 rows=60001 loops=1)
Planning Time: 0.071 ms
Execution Time: 4084.733 ms
~~END~~

Tooling impact -
None
Client tests -
None

Check List

Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is under the terms of the Apache 2.0 and PostgreSQL licenses, and grant any person obtaining a copy of the contribution permission to relicense all or a portion of my contribution to the PostgreSQL License solely to contribute all or a portion of my contribution to the PostgreSQL open source project.

For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Jake Owen <[email protected]>

forestkeeper · 2023-11-09T18:58:30Z

contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql

+AS
+$body$
+BEGIN
+    return sys.datediff_internal(datepart, startdate::TIMESTAMP, enddate::TIMESTAMP);


Does a explicit type change happen in here ?

Signed-off-by: Jake Owen <[email protected]>

forestkeeper · 2023-11-09T20:12:22Z

contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql

+DECLARE
+	timezone INTEGER;
+BEGIN
+	timezone = sys.babelfish_get_datetimeoffset_tzoffset(startdate)::INTEGER * 2;


Why this part is not moved into c impl ?

forestkeeper · 2023-11-09T20:20:25Z

contrib/babelfishpg_common/src/datetimeoffset.c

+				break;
+		}
+	} else {
+		elog(ERROR, "the datepart %s is not supported by function dateadd for datatype datetimeoffset", lowunits);


The original msg on the script impl is :

RAISE EXCEPTION '"%" is not a recognized dateadd option.', datepart;

Signed-off-by: Jake Owen <[email protected]>

forestkeeper · 2023-11-09T20:21:45Z

contrib/babelfishpg_common/src/datetimeoffset.c

+				interval = (Interval *) DirectFunctionCall7(make_interval, 0, 0, 0, 0, 0, 0, Float8GetDatum((float) num * 0.000000001));
+				break;
+			default:
+				elog(ERROR, "the datepart %s is not supported by function dateadd for datatype datetimeoffset",


Same as the error msg is different
and could you combine too err out places together

forestkeeper · 2023-11-09T20:27:12Z

contrib/babelfishpg_tsql/sql/upgrades/babelfishpg_tsql--3.3.0--3.4.0.sql

+AS
+$body$
+BEGIN
+    return sys.datediff_internal(datepart, startdate::TIMESTAMP, enddate::TIMESTAMP);


The original impl, the datediff internal script version , it's passing startdate and enddate as PG_CATALOG.date into datediff_internal.
This implement, just did an extra explicit type conversion in here, and I don't understand why it should do that.

forestkeeper · 2023-11-09T20:32:58Z

contrib/babelfishpg_common/src/datetime.c

+			case DTK_YEAR:
+				if(dttype == TIME) {
+					elog(ERROR, "the datepart %s is not supported by date function dateadd for data type time",


The original err msg should be :

The datepart % is not supported by date function dateadd for data type date.{datepart}

slight different from this implement

forestkeeper · 2023-11-09T20:33:40Z

contrib/babelfishpg_common/src/datetime.c

+		}
+	} else {
+		elog(ERROR, "the datepart %s is not supported by date function dateadd for data type %s",


And can we make the errmsg in the same code appear place , not repeat so many times

forestkeeper · 2023-11-09T20:35:15Z

contrib/babelfishpg_common/src/datetime.c

+		ereport(ERROR,
+				(errcode(ERRCODE_DATETIME_VALUE_OUT_OF_RANGE),
+				 errmsg("data out of range for %s"), datetypeName(dttype)));


The original sql script the err msg should be :

RAISE EXCEPTION '''%'' is not a recognized dateadd option.', datepart;

forestkeeper · 2023-11-09T20:38:34Z

test/JDBC/expected/BABEL-2812-vu-verify.out


-~~ERROR (Message: integer out of range)~~
+~~ERROR (Message: The datediff function resulted in an overflow. The number of dateparts separating two date/time instances is too large. Try to use datediff with a less precise datepart)~~


Is that expected ?

forestkeeper · 2023-11-09T20:41:12Z

Would you also mention how much percentage performance gain we have for this c impl ?

forestkeeper · 2023-11-10T03:47:58Z

contrib/babelfishpg_common/src/datetime.c

+					break;
+				default:
+					elog(ERROR, "the datepart \"%s\" is not supported by function datediff for datatype date", lowunits);


Pls also make sure this error msg is the same as previous

forestkeeper · 2023-11-10T03:48:29Z

contrib/babelfishpg_common/src/datetime.c

+
+	if(overflow) {
+		elog(ERROR, "The datediff function resulted in an overflow. The number of dateparts separating two date/time instances is too large. Try to use datediff with a less precise datepart");


Where is the error msg comes from ?

forestkeeper · 2023-11-10T03:50:28Z

contrib/babelfishpg_common/src/datetime.c

+	bool       overflow = false;
+
+	ok1 = timestamp2tm(timestamp1, NULL, tm1, &fsec1, NULL, NULL);


Can we make a better name for ok1, ok2 ?

forestkeeper · 2023-11-10T03:51:30Z

contrib/babelfishpg_common/src/datetime.c

+	int32 microsecdiff;
+	struct pg_tm tt1,
+			   *tm1 = &tt1;


To define a tt1, and make a pointer to tt1, looks weired, do we have a better way to define those ? And also, pls give a meaningful names instead of tm1, tm2 .

Signed-off-by: Jake Owen <[email protected]>

forestkeeper · 2023-11-15T23:37:27Z

contrib/babelfishpg_common/src/datetime.c

 	}

+	if(!validDateDiff) {
+		elog(ERROR, "The datepart %s is not a recognized datediff option.", lowunits);


I think the original error didn't have 'The'

RAISE EXCEPTION '"%" is not a recognized datediff option.', datepart;

forestkeeper · 2023-11-15T23:42:15Z

contrib/babelfishpg_common/src/datetime.c

+	}
+	if(overflow) {
+		elog(ERROR, "The datediff function resulted in an overflow. The number of dateparts separating two date/time instances is too large. Try to use datediff with a less precise datepart");


And also , we should use ereport to error out in c impl, and to make sure the transaction behavior is the same compare to sql server for this error msg

forestkeeper · 2023-11-15T23:43:03Z

contrib/babelfishpg_common/src/datetime.c

+
+	if(!validDateDiff) {
+		elog(ERROR, "The datepart %s is not a recognized datediff option.", lowunits);


Should add test cases for each error out and make sure transaction behavior is the same as sql server ( when error out, shouldn't abort the current transaction )

forestkeeper · 2023-11-15T23:44:28Z

contrib/babelfishpg_common/src/datetime.c

+			case DTK_MICROSEC:
+				if(dttype == SMALLDATETIME || dttype == DATETIME || dttype == DATE) {
+					elog(ERROR, "The datepart %s is not supported by date function dateadd for data type %s.",


Would you reformat this part of code, to make sure all this error out in the same place and the err msg is also not the same cause it has an additional 'The' in the msg body.

Signed-off-by: Jake Owen <[email protected]>

forestkeeper · 2023-11-17T01:22:14Z

contrib/babelfishpg_common/src/datetime.c

+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("The datepart %s is not supported by date function %s for data type %s.", lowunits, "dateadd", "date")));


Those are all very similar err msg :

ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("The datepart %s is not supported by date function %s for data type %s.", lowunits, "dateadd", "date"))); }

please make sure we don't repeat this code everywhere

forestkeeper · 2023-11-17T01:25:05Z

test/JDBC/expected/babel_datetime-vu-verify.out

@@ -371,13 +371,6 @@ datetime
 2016-12-27 00:25:29.0
 ~~END~~

-- out of range
-select dateadd(year, 150, cast('9900-12-26 23:29:29' as datetime))


Why we delete this test case ?

Signed-off-by: Jake Owen <[email protected]>

…sh-for-postgresql#1998) This change reimplements the datediff, datediff_big, and dateadd functions in C to improve performance by 65% compared to the original implementation. Task: BABEL-4496 Signed-off-by: Jake Owen <[email protected]>

Jake Owen added 9 commits November 8, 2023 19:24

Reimplement datediff and dateadd in c to improve performance

f2376cb

Signed-off-by: Jake Owen <[email protected]>

Remove comments

6fc01d1

Signed-off-by: Jake Owen <[email protected]>

Updated upgrade files and fix day in date

5ee47a5

Signed-off-by: Jake Owen <[email protected]>

Merge branch 'BABEL_3_X_DEV' into jira-babel-4496

26c4de4

Updated testing

2d4689d

Signed-off-by: Jake Owen <[email protected]>

Fix function removal

ed0d56c

Signed-off-by: Jake Owen <[email protected]>

Updated dependency file

602dfd9

Signed-off-by: Jake Owen <[email protected]>

Change upgrade drop function call

e97b681

Signed-off-by: Jake Owen <[email protected]>

Update error handling

8aa2cf1

Signed-off-by: Jake Owen <[email protected]>

forestkeeper reviewed Nov 9, 2023

View reviewed changes

Fix testing output diff

4319c8a

Signed-off-by: Jake Owen <[email protected]>

forestkeeper reviewed Nov 9, 2023

View reviewed changes

Fix datetime out of range message

f2429fc

Signed-off-by: Jake Owen <[email protected]>

forestkeeper reviewed Nov 9, 2023

View reviewed changes

forestkeeper reviewed Nov 10, 2023

View reviewed changes

Jake Owen added 5 commits November 14, 2023 17:35

Fix error messages

781b906

Signed-off-by: Jake Owen <[email protected]>

Remove changes from upgrade script temp

10a3058

Signed-off-by: Jake Owen <[email protected]>

Merge branch 'BABEL_3_X_DEV' into jira-babel-4496

33a7851

Remove expected dependency

1d43425

Signed-off-by: Jake Owen <[email protected]>

Update expected tests

11d8e3b

Signed-off-by: Jake Owen <[email protected]>

Jake Owen added 8 commits November 14, 2023 23:02

Drop functions to avoid error

36f68c6

Signed-off-by: Jake Owen <[email protected]>

Fix syntax error

0d7e504

Signed-off-by: Jake Owen <[email protected]>

Fix incorrect errcode

3340558

Signed-off-by: Jake Owen <[email protected]>

Merge branch 'BABEL_3_X_DEV' into jira-babel-4496

bf70129

Fix errocode output in test files

a0cee91

Signed-off-by: Jake Owen <[email protected]>

Resolve format_datetime mvu failure

3d1e5a3

Signed-off-by: Jake Owen <[email protected]>

Modify error calls in datediff

cefaae3

Signed-off-by: Jake Owen <[email protected]>

Unify error calls and cleanup test files

b85969c

Signed-off-by: Jake Owen <[email protected]>

forestkeeper reviewed Nov 15, 2023

View reviewed changes

Jake Owen added 7 commits November 16, 2023 16:21

Merge branch 'BABEL_3_X_DEV' into jira-babel-4496

d5f039f

Add appropriate error codes

0d37ae3

Signed-off-by: Jake Owen <[email protected]>

Fix merge conflict

5ccd559

Updated expected output error messages

1593b72

Signed-off-by: Jake Owen <[email protected]>

Updated output files with proper error codes

4256536

Signed-off-by: Jake Owen <[email protected]>

Update TestErrorHelperFunctionsUpgrade-vu-verify.out

329a733

Signed-off-by: Jake Owen <[email protected]>

Empty-Commit to restart test

77a21cf

forestkeeper reviewed Nov 17, 2023

View reviewed changes

Jake Owen added 4 commits November 17, 2023 14:52

Consolidate error messages and add test case

7c5c9d8

Signed-off-by: Jake Owen <[email protected]>

Merge branch 'BABEL_3_X_DEV' into jira-babel-4496

17147ba

Remove datediff_internal_date from new schedules

81491c7

Signed-off-by: Jake Owen <[email protected]>

Remove tests from schedule

f819833

Signed-off-by: Jake Owen <[email protected]>

forestkeeper approved these changes Nov 17, 2023

View reviewed changes

forestkeeper merged commit d5d1d05 into babelfish-for-postgresql:BABEL_3_X_DEV Nov 17, 2023
29 checks passed

Jakeowen1 mentioned this pull request Nov 20, 2023

Add tests back to 15.5 schedule #2038

Closed

1 task

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Reimplement datediff and dateadd in c to improve performance #1998

Reimplement datediff and dateadd in c to improve performance #1998

Jakeowen1 commented Nov 8, 2023 •

edited

Loading

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper Nov 9, 2023

forestkeeper commented Nov 9, 2023

forestkeeper Nov 10, 2023

forestkeeper Nov 10, 2023

forestkeeper Nov 10, 2023

forestkeeper Nov 10, 2023

forestkeeper Nov 15, 2023

forestkeeper Nov 15, 2023

forestkeeper Nov 15, 2023

forestkeeper Nov 15, 2023

forestkeeper Nov 17, 2023

forestkeeper Nov 17, 2023


		~~ERROR (Message: integer out of range)~~
		~~ERROR (Message: The datediff function resulted in an overflow. The number of dateparts separating two date/time instances is too large. Try to use datediff with a less precise datepart)~~


		if(overflow) {
		elog(ERROR, "The datediff function resulted in an overflow. The number of dateparts separating two date/time instances is too large. Try to use datediff with a less precise datepart");

		bool overflow = false;

		ok1 = timestamp2tm(timestamp1, NULL, tm1, &fsec1, NULL, NULL);


		if(!validDateDiff) {
		elog(ERROR, "The datepart %s is not a recognized datediff option.", lowunits);

Reimplement datediff and dateadd in c to improve performance #1998

Reimplement datediff and dateadd in c to improve performance #1998

Conversation

Jakeowen1 commented Nov 8, 2023 • edited Loading

Description

Issues Resolved

Test Scenarios Covered

Check List

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

forestkeeper commented Nov 9, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Jakeowen1 commented Nov 8, 2023 •

edited

Loading