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

Support time series in PlyType #316

Merged
merged 1 commit into from
Oct 2, 2024
Merged

Support time series in PlyType #316

merged 1 commit into from
Oct 2, 2024

Conversation

pengz-imply
Copy link
Contributor

This PR makes the necessary changes to support the TIME_SERIES type.

Copy link
Contributor

@jgoz jgoz left a comment

Choose a reason for hiding this comment

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

This looks fine, but I wonder if it is complete. Do we need a TimeSeries datatype? And should it get included in Dataset, formatting, inflating, etc.?

I don't know the answer to these questions, but the last new type we added, IP, included a datatype Ip.

@pengz-imply
Copy link
Contributor Author

pengz-imply commented Oct 2, 2024

This looks fine, but I wonder if it is complete. Do we need a TimeSeries datatype? And should it get included in Dataset, formatting, inflating, etc.?

I don't know the answer to these questions, but the last new type we added, IP, included a datatype Ip.

Good question! I'm not very familiar with Dataset, but from what I found, it is related to querying data using Plywood. Currently in Pivot, querying time series data is only supported for SQL query construction using druid-query-toolkit without needing Plywood. I don't have a plan for supporting querying time series data in Plywood, Therefore, I only made minimal changes in Plywood for Pivot to recognize ingested time series columns as "TIME_SERIES" from introspection, and that's the extent of the purpose.

It seems that the 'dataType' is used to create various expressions. These expressions will not be covered for time series, so I think it's fine not to add time series as a data type.

@pengz-imply pengz-imply changed the title Support time series data type Support time series in PlyType Oct 2, 2024
@jgoz jgoz merged commit ca4458d into master Oct 2, 2024
1 check passed
@jgoz jgoz deleted the support-time-series-type branch October 2, 2024 17:28
@jgoz
Copy link
Contributor

jgoz commented Oct 2, 2024

@pengz-imply Published as 0.35.5

@pengz-imply
Copy link
Contributor Author

@pengz-imply Published as 0.35.5

Thank you!

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.

2 participants