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 datetime parsing for executing UDFs at timestamps. #653

Merged
merged 3 commits into from
Sep 19, 2024

Conversation

thetorpedodog
Copy link
Contributor

This change adds the initial step for executing UDFs at timestamps. To allow users to specify a timestamp of a registered UDF to use, we add @ followed by a timestamp to the name of the UDF.

For example, to execute the version of my/udf as it was as of 2023-06-17 at 07:16 UTC, they would execute:

"my/udf@2023-06-17 07:16"

This is unambiguous and avoids the need to add a new parameter, which would be difficult to name and could shadow parameters in user code.


Setting this up now separately so that discussion of how we specify old versions isn't caught up in the rest of the implementation details.

This change adds the initial step for executing UDFs at timestamps.
To allow users to specify a timestamp of a registered UDF to use,
we add `@` followed by a timestamp to the name of the UDF.

For example, to execute the version of `my/udf` as it was as of
2023-06-17 at 07:16 UTC, they would execute:

    "my/udf@2023-06-17 07:16"

This is unambiguous and avoids the need to add a new parameter, which
would be difficult to name and could shadow parameters in user code.
@sgillies
Copy link
Collaborator

@thetorpedodog I think @ as a delimeter works (it's like what pip uses for tags and commits). What would you think about eliminating the blank between date and time (whitespace being a source of bugs) and replacing it with a T ala https://datatracker.ietf.org/doc/html/rfc3339#section-5.8?

@thetorpedodog
Copy link
Contributor Author

@thetorpedodog I think @ as a delimeter works (it's like what pip uses for tags and commits). What would you think about eliminating the blank between date and time (whitespace being a source of bugs) and replacing it with a T ala https://datatracker.ietf.org/doc/html/rfc3339#section-5.8?

This code accepts both and T as separators. My thought there is that I find 2024-09-18 14:03 more readable than 2024-09-18T14:03, but the latter is the standard. That said, I think it might be better to use the T version as the canonical form for parsing, so instead of changing T to , I’ll change to T.

One more idea I have: pip and its ilk allow spaces around the delimiter (this @ 2024-09-18T14:03), so it might also be good to do that. This is in a separate commit so we can take it separately.

Copy link
Collaborator

@sgillies sgillies left a comment

Choose a reason for hiding this comment

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

👍 My only concern about the whitespace is UX: identifiers without whitespace are easy to select, copy, and share.

@sgillies sgillies merged commit 137cd14 into main Sep 19, 2024
18 checks passed
@sgillies sgillies deleted the timestamped-udfs branch September 19, 2024 15:07
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