Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for MySQL Date/Time/TimeStamp/DateTime date type transformation #5190

Merged
merged 7 commits into from
Nov 15, 2024

Conversation

dinujoh
Copy link
Member

@dinujoh dinujoh commented Nov 14, 2024

Description

Add support for MySQL Date/Time/TimeStamp/DateTime date type transformation between binlog and S3 export formats to OpenSearch.

MySQL binlog represents temporal types as follows:

  • DATE: long value representing milliseconds since epoch (1970-01-01)
  • TIME: long value representing milliseconds since epoch (1970-01-01 00:00:00)
  • DATETIME: long value representing microseconds since epoch (1970-01-01 00:00:00)
  • YEAR: 4-digit year value (Example: 2024)

S3 export formats:

  • DATE: "yyyy-MM-dd" (Example: "2024-01-15")
  • TIME: "HH:mm:ss" (Example: "14:30:00")
  • DATETIME: "yyyy-MM-dd HH:mm:ss[.SSSSSS]" (Example: "2024-01-15 14:30:00.123456")
    Note: Fractional seconds are optional for DATETIME
  • YEAR: "yyyy" (Example: "2024")

Issues Resolved

Contributes to #4561

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • Commits are signed with a real name per the DCO

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: Dinu John <[email protected]>
Signed-off-by: Dinu John <[email protected]>
chenqi0805
chenqi0805 previously approved these changes Nov 14, 2024
Comment on lines 91 to 92
final Long dateEpoch = parseDateTimeStrAsEpoch(dateStr);
if (dateEpoch != null) return dateEpoch;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the return value here epoch in millis or in days, since "Date from binlog is long value representing days since epoch"?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

@oeyh oeyh Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In that case, we need to convert it to epoch millis to be consistent with the conversion below for export data?

Copy link
Member Author

@dinujoh dinujoh Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated documentation

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense. Thanks for clarifying!

Comment on lines 106 to 107
final Long dateTimeEpoch = parseDateTimeStrAsEpoch(dateTimeStr);
if (dateTimeEpoch != null) return dateTimeEpoch;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, is the return value epoch in millis or epoch in micros?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's epoch seconds


private static Stream<Arguments> provideDateTestCases() throws ParseException {
return Stream.of(
Arguments.of("2023-12-25", getEpochMillisFromDateStr("2023-12-25")),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We may want to add a test case with a string of long value as input.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

}

private static Stream<Arguments> provideTimeTestCases() {
return Stream.of(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly, we may want to add a test case with a string of long value as input.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added

Signed-off-by: Dinu John <[email protected]>
Copy link
Collaborator

@oeyh oeyh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@dinujoh dinujoh merged commit 8e6874d into opensearch-project:main Nov 15, 2024
46 of 47 checks passed
kkondaka pushed a commit to kkondaka/kk-data-prepper-f2 that referenced this pull request Nov 18, 2024
…mation (opensearch-project#5190)

* Add support for MySQL Date/Time/TimeStamp/DateTime date type transformation to OpenSearch

Signed-off-by: Dinu John <[email protected]>

* Configure BinlogClient to deserialize data and time as long

Signed-off-by: Dinu John <[email protected]>

* Add documentation

Signed-off-by: Dinu John <[email protected]>

* Update documentation

Signed-off-by: Dinu John <[email protected]>

* Update documentation

Signed-off-by: Dinu John <[email protected]>

* Update unit test and documentation

Signed-off-by: Dinu John <[email protected]>

* Update documentation

Signed-off-by: Dinu John <[email protected]>

---------

Signed-off-by: Dinu John <[email protected]>
Signed-off-by: Kondaka <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants