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

feat: add PARSE_DATE and FORMAT_DATE functions #7733

Merged
merged 5 commits into from
Jul 1, 2021

Conversation

jzaralim
Copy link
Contributor

Description

Enables DATE data for UDFs and adds the PARSE_DATE and FORMAT_DATE functions + docs.

Testing done

QTT + unit tests

Reviewer checklist

  • Ensure docs are updated if necessary. (eg. if a user visible feature is being added or changed).
  • Ensure relevant issues are linked (description should include text like "Fixes #")

@jzaralim jzaralim requested a review from spena June 26, 2021 00:51
@jzaralim jzaralim requested review from JimGalasyn and a team as code owners June 26, 2021 00:51
Copy link
Member

@JimGalasyn JimGalasyn left a comment

Choose a reason for hiding this comment

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

LGTM, with a couple of suggestions. Also needs to be cherry-[picked to the 0.20.x-ksqldb branch.

docs/developer-guide/ksqldb-reference/scalar-functions.md Outdated Show resolved Hide resolved
}
try {
final DateTimeFormatter formatter = formatters.get(formatPattern);
return LocalDate.ofEpochDay(TimeUnit.MILLISECONDS.toDays(date.getTime())).format(formatter);
Copy link
Member

Choose a reason for hiding this comment

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

I wonder what if formatPattern contains time characters? Would those be part of the final String?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a test for this.

final Object result = udf.formatDate(Date.valueOf("2014-11-09"), "yyyy-dd-MM'Fred'");

// Then:
assertThat(result, is("2014-09-11Fred"));
Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure about this. Should we support embedded chars in the final date string?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It was supported in the old DateToString function:

Copy link
Member

Choose a reason for hiding this comment

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

Just tested Mysql and it supports this too. Perhaps it is allowed in order to write the format you want, i.e. dates yyyy/MM/dd or yyyy-MM-dd or Today is Year = yyyy Month = MM Day = dd

Copy link
Member

@spena spena left a comment

Choose a reason for hiding this comment

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

LGTM

@jzaralim jzaralim merged commit 5a64ed7 into confluentinc:master Jul 1, 2021
@jzaralim jzaralim deleted the parse-format-date branch July 1, 2021 02:06
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