Add functions `PERIOD_ADD` and `PERIOD_DIFF`. #130

Yury-Fridlyand · 2022-10-07T03:31:59Z

Signed-off-by: Yury-Fridlyand [email protected]

Description

I referred to MySQL docs and tried to reproduce MySQL v.8.0.30 behavior as a reference.

New function: `PERIOD_ADD`.

Changes

Adds N months to period P (in the format YYMM or YYYYMM). Returns a value in the format YYYYMM.

Fully compliant with MySQL.

Signature

(LONG, LONG) -> LONG

Future changes (TODOs):

Update if and after opensearch-project#862

Notes

Not documented, but with experiments I discovered that

1 or 2 digits year is interpreted as 2000 + x.
2 digit year lower that 70 interpreted as 20xx, higher than 70 - as 19xx

Test queries:

SELECT PERIOD_ADD(200801, 2), PERIOD_ADD(200801, -12);
SELECT PERIOD_ADD(111, 2), PERIOD_ADD(1111, 2), PERIOD_ADD(1111, 2);
SELECT PERIOD_ADD(7011, 2), PERIOD_ADD(6911, 2);

New function: `PERIOD_DIFF`.

Changes

Returns the number of months between periods P1 and P2. P1 and P2 should be in the format YYMM or YYYYMM.

Fully compliant with MySQL.

Signature

(LONG, LONG) -> LONG

Future changes (TODOs):

Update if and after opensearch-project#862

Notes

Not documented, but with experiments I discovered that

1 or 2 digits year is interpreted as 2000 + x.
2 digit year lower that 70 interpreted as 20xx, higher than 70 - as 19xx

Test queries:

SELECT PERIOD_DIFF(200802, 200703), PERIOD_DIFF(200802, 201003);
SELECT PERIOD_DIFF(7011, 6911);

Test data

I found that first 6 rows from date0, time0, time1, datetime0 are good for testing - these columns have different data types in MySQL. In OpenSearch SQL all [date][time] columns have timestamp type, so I use CAST for clear testing.

data

mysql> show fields from Calcs where field IN ('date0', 'time0', 'time1', 'datetime0');
+-----------+-----------+------+-----+---------+-------+
| Field     | Type      | Null | Key | Default | Extra |
+-----------+-----------+------+-----+---------+-------+
| date0     | date      | YES  |     | NULL    |       |
| time0     | datetime  | YES  |     | NULL    |       |
| time1     | time      | YES  |     | NULL    |       |
| datetime0 | timestamp | YES  |     | NULL    |       |
+-----------+-----------+------+-----+---------+-------+
4 rows in set (0.00 sec)
mysql> select date0, time0, time1, datetime0 from calcs;
+------------+---------------------+----------+---------------------+
| date0      | time0               | time1    | datetime0           |
+------------+---------------------+----------+---------------------+
| 2004-04-15 | 1899-12-30 21:07:32 | 19:36:22 | 2004-07-09 10:17:35 |
| 1972-07-04 | 1900-01-01 13:48:48 | 02:05:25 | 2004-07-26 12:30:34 |
| 1975-11-12 | 1900-01-01 18:21:08 | 09:33:31 | 2004-08-02 07:59:23 |
| 2004-06-04 | 1900-01-01 18:51:48 | 22:50:16 | 2004-07-05 13:14:20 |
| 2004-06-19 | 1900-01-01 15:01:19 | NULL     | 2004-07-28 23:30:22 |
| NULL       | 1900-01-01 08:59:39 | 19:57:33 | 2004-07-22 00:30:23 |
| NULL       | 1900-01-01 07:37:48 | NULL     | 2004-07-28 06:54:50 |
| NULL       | 1900-01-01 19:45:54 | 19:48:23 | 2004-07-12 17:30:16 |
| NULL       | 1900-01-01 09:00:59 | 22:20:14 | 2004-07-04 22:49:28 |
| NULL       | 1900-01-01 20:36:00 | NULL     | 2004-07-23 21:13:37 |
| NULL       | 1900-01-01 01:31:32 | 00:05:57 | 2004-07-14 08:16:44 |
| NULL       | 1899-12-30 22:15:40 | 04:40:49 | 2004-07-25 15:22:26 |
| NULL       | 1900-01-01 13:53:46 | 04:48:07 | 2004-07-17 14:01:56 |
| NULL       | 1900-01-01 04:57:51 | NULL     | 2004-07-19 22:21:31 |
| NULL       | 1899-12-30 22:42:43 | 18:58:41 | 2004-07-31 11:57:52 |
| NULL       | 1899-12-30 22:24:08 | NULL     | 2004-07-14 07:43:00 |
| NULL       | 1900-01-01 11:58:29 | 12:33:57 | 2004-07-28 12:34:28 |
+------------+---------------------+----------+---------------------+
17 rows in set (0.00 sec)

Check List

New functionality includes testing.
- All tests pass, including unit test, integration test and doctest
New functionality has been documented.
- New functionality has javadoc added
- New functionality has user manual doc added
Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Yury-Fridlyand <[email protected]>

codecov · 2022-10-07T03:43:13Z

Codecov Report

Merging #130 (cc8edf6) into integ-datetime-period-functions (057fa44) will decrease coverage by 2.84%.
The diff coverage is 100.00%.

@@                          Coverage Diff                          @@
##             integ-datetime-period-functions     #130      +/-   ##
=====================================================================
- Coverage                              97.87%   95.03%   -2.85%     
- Complexity                              3020     3029       +9     
=====================================================================
  Files                                    284      294      +10     
  Lines                                   7425     8111     +686     
  Branches                                 475      597     +122     
=====================================================================
+ Hits                                    7267     7708     +441     
- Misses                                   157      349     +192     
- Partials                                   1       54      +53

Flag	Coverage Δ
query-workbench	`62.76% <ø> (?)`
sql-engine	`97.88% <100.00%> (+<0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...arch/sql/expression/datetime/DateTimeFunction.java	`100.00% <100.00%> (ø)`
...h/sql/expression/function/BuiltinFunctionName.java	`100.00% <100.00%> (ø)`
workbench/public/utils/PanelWrapper.tsx	`100.00% <0.00%> (ø)`
workbench/public/application.tsx	`0.00% <0.00%> (ø)`
workbench/public/components/app.tsx	`0.00% <0.00%> (ø)`
...h/public/components/QueryLanguageSwitch/Switch.tsx	`85.71% <0.00%> (ø)`
workbench/public/components/PPLPage/PPLPage.tsx	`56.52% <0.00%> (ø)`
...ch/public/components/QueryResults/QueryResults.tsx	`61.60% <0.00%> (ø)`
workbench/public/components/Header/Header.tsx	`100.00% <0.00%> (ø)`
... and 3 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

GumpacG

LGTM

acarbonetto · 2022-10-13T00:30:57Z

core/src/main/java/org/opensearch/sql/expression/datetime/DateTimeFunction.java

@@ -68,6 +69,7 @@
 * 2) the implementation should rely on ExprValue.
 */
 @UtilityClass
+@SuppressWarnings("unchecked")


can we move this to the specific lines that fail instead of suppressing warnings everywhere in the file?

acarbonetto · 2022-10-13T00:33:50Z

core/src/test/java/org/opensearch/sql/expression/datetime/PeriodFunctionsTest.java

+   */
+  public static Stream<Arguments> getInvalidTestData() {
+    return Stream.of(
+      Arguments.of(0),


do we need to try negative numbres

I think we should try negative numbers so we're covering all possible cases

Thank you, fixed in cc8edf6.

acarbonetto · 2022-10-13T00:34:45Z

integ-test/src/test/java/org/opensearch/sql/ppl/DateTimeFunctionIT.java

 import org.json.JSONObject;
 import org.junit.jupiter.api.Test;
 import org.opensearch.sql.common.utils.StringUtils;

+@SuppressWarnings("unchecked")


what is this suppressing?

Every call for verifySome in every test in this file (91 hits).

MitchellGale

lgtm

MitchellGale · 2022-10-17T18:12:49Z

core/src/test/java/org/opensearch/sql/expression/datetime/PeriodFunctionsTest.java

+   */
+  public static Stream<Arguments> getTestDataForPeriodAdd() {
+    return Stream.of(
+        Arguments.of(1, 3, 200004), // Jan 2000 + 3


Is this April 2000 instead of Jan 2000?

Last argument is the expected result value.

Signed-off-by: Yury-Fridlyand <[email protected]>

Add XYPoint Field Type to index and query documents that contains cartesian points

Add functions PERIOD_ADD and PERIOD_DIFF.

8ece4ea

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand requested review from GumpacG, forestmvey, GabeFernandez310, MaxKsyunz, acarbonetto and MitchellGale October 7, 2022 03:31

Yury-Fridlyand mentioned this pull request Oct 7, 2022

SQL ingestion and ops - standup opensearch-project/sql#791

Open

GumpacG reviewed Oct 8, 2022

View reviewed changes

acarbonetto approved these changes Oct 14, 2022

View reviewed changes

MitchellGale approved these changes Oct 17, 2022

View reviewed changes

forestmvey approved these changes Oct 17, 2022

View reviewed changes

MaxKsyunz approved these changes Oct 17, 2022

View reviewed changes

GabeFernandez310 approved these changes Oct 17, 2022

View reviewed changes

GumpacG approved these changes Oct 17, 2022

View reviewed changes

Minor test improvements according to PR feedback.

cc8edf6

Signed-off-by: Yury-Fridlyand <[email protected]>

Yury-Fridlyand merged commit f9072b2 into integ-datetime-period-functions Oct 19, 2022

Yury-Fridlyand deleted the dev-datetime-period-functions branch October 19, 2022 19:34

Yury-Fridlyand mentioned this pull request Oct 19, 2022

Add functions PERIOD_ADD and PERIOD_DIFF. opensearch-project/sql#933

Merged

6 tasks

andy-k-improving pushed a commit that referenced this pull request Nov 16, 2024

Merge pull request #130 from opensearch-project/feature/xypoint

3288818

Add XYPoint Field Type to index and query documents that contains cartesian points

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add functions `PERIOD_ADD` and `PERIOD_DIFF`. #130

Add functions `PERIOD_ADD` and `PERIOD_DIFF`. #130

Yury-Fridlyand commented Oct 7, 2022 •

edited

Loading

codecov bot commented Oct 7, 2022 •

edited

Loading

GumpacG left a comment

acarbonetto Oct 13, 2022

acarbonetto Oct 13, 2022

GabeFernandez310 Oct 17, 2022

Yury-Fridlyand Oct 18, 2022

acarbonetto Oct 13, 2022

Yury-Fridlyand Oct 18, 2022 •

edited

Loading

MitchellGale left a comment

MitchellGale Oct 17, 2022

Yury-Fridlyand Oct 18, 2022

Add functions PERIOD_ADD and PERIOD_DIFF. #130

Add functions PERIOD_ADD and PERIOD_DIFF. #130

Conversation

Yury-Fridlyand commented Oct 7, 2022 • edited Loading

Description

New function: PERIOD_ADD.

Signature

Future changes (TODOs):

Notes

Test queries:

New function: PERIOD_DIFF.

Signature

Future changes (TODOs):

Notes

Test queries:

Test data

Check List

codecov bot commented Oct 7, 2022 • edited Loading

Codecov Report

GumpacG left a comment

Choose a reason for hiding this comment

acarbonetto Oct 13, 2022

Choose a reason for hiding this comment

acarbonetto Oct 13, 2022

Choose a reason for hiding this comment

GabeFernandez310 Oct 17, 2022

Choose a reason for hiding this comment

Yury-Fridlyand Oct 18, 2022

Choose a reason for hiding this comment

acarbonetto Oct 13, 2022

Choose a reason for hiding this comment

Yury-Fridlyand Oct 18, 2022 • edited Loading

Choose a reason for hiding this comment

MitchellGale left a comment

Choose a reason for hiding this comment

MitchellGale Oct 17, 2022

Choose a reason for hiding this comment

Yury-Fridlyand Oct 18, 2022

Choose a reason for hiding this comment

Add functions `PERIOD_ADD` and `PERIOD_DIFF`. #130

Add functions `PERIOD_ADD` and `PERIOD_DIFF`. #130

Yury-Fridlyand commented Oct 7, 2022 •

edited

Loading

New function: `PERIOD_ADD`.

New function: `PERIOD_DIFF`.

codecov bot commented Oct 7, 2022 •

edited

Loading

Yury-Fridlyand Oct 18, 2022 •

edited

Loading