add core functions make_timestamp_ns(nanos) and epoch_ns(timestamp_ts) #14930

andreimatei · 2024-11-20T19:26:25Z

See individual commits.

andreimatei · 2024-11-20T19:30:11Z

I have a feeling that the new make_timestamp_ns function should be listed in extension_entries.hpp, but I have failed to generate that file correctly. I've tried invoking it in different ways, but no matter what I do, the generate_extensions_function.py script deletes all core_functions entries for me. Perhaps I could get help with the right entry. Thanks!

hawkfish

Very minor stuff.

hawkfish · 2024-11-20T20:28:59Z

src/include/duckdb/common/types/timestamp.hpp

@@ -24,6 +24,8 @@ struct dtime_tz_t; // NOLINT

 //! Type used to represent a TIMESTAMP. timestamp_t holds the microseconds since 1970-01-01.
 struct timestamp_t { // NOLINT
+	// NOTE: The unit of value is microseconds for timestamp_t, but it can de


hawkfish · 2024-11-20T20:29:46Z

test/sql/function/timestamp/test_date_part.test

@@ -961,3 +961,9 @@ statement error
 WITH parts(p) AS (VALUES (['year', 'month', 'day']), (['hour', 'minute', 'microsecond']))
 SELECT DATE_PART(p, ts) FROM parts, timestamps;
 ----
+
+# epoch_ns has nanosecond precision on TIMESTAMP_TS


hawkfish · 2024-11-20T20:34:29Z

extension/core_functions/scalar/date/make_date.cpp

@@ -98,7 +98,7 @@ struct MakeTimestampOperator {

 	template <typename T, typename RESULT_TYPE>
 	static RESULT_TYPE Operation(T micros) {
-		return timestamp_t(micros);
+		return RESULT_TYPE(micros);


Maybe we should rename micros to value or something?

separated change to its own commit and done

hawkfish · 2024-11-20T20:37:16Z

cc @hawkfish

I have a feeling that the new make_timestamp_ns function should be listed in extension_entries.hpp, but I have failed to generate that file correctly. I've tried invoking it in different ways, but no matter what I do, the generate_extensions_function.py script deletes all core_functions entries for me. Perhaps I could get help with the right entry. Thanks!

You've got me - this has changed recently and I've been on holiday. @taniabogatsch ?

andreimatei · 2024-11-20T23:23:43Z

You've got me - this has changed recently and I've been on holiday. @taniabogatsch ?

Actually, a failing test on CI told me what to add to that file. Done.

taniabogatsch

@hawkfish, indeed, there have been a few changes to the s, ms, ns types. See #14818.

taniabogatsch · 2024-11-22T12:22:23Z

src/include/duckdb/common/types/timestamp.hpp

@@ -189,6 +191,7 @@ class Timestamp {
 	DUCKDB_API static int64_t GetEpochMicroSeconds(timestamp_t timestamp);
 	//! Convert a timestamp to epoch (in nanoseconds)
 	DUCKDB_API static int64_t GetEpochNanoSeconds(timestamp_t timestamp);
+	DUCKDB_API static int64_t GetEpochNanoSeconds(timestamp_ns_t timestamp);


This is the same as directly accessing value, no? I'm unsure if we need it, but I'm also okay with adding it.

This is the same as directly accessing value, no?

Yes. But so is GetEpochMicroSeconds(timestamp_t timestamp) -- and that function exists.
So, if it's up to me, I'd leave it. But I don't know this code at all, so...

Yeah, that's fine, we can leave it. 👍

taniabogatsch · 2024-11-22T12:23:04Z

test/sql/function/timestamp/make_date.test

+# From nanoseconds
+query II
+SELECT make_timestamp_ns(0), make_timestamp_ns(1684509234845000123);
+----
+1970-01-01 00:00:00	2023-05-19 15:13:54.845000123
+


Should we add tests for min and max values?

Do you mean make_timestamp_ns(-9,223,372,036,854,775,808) and make_timestamp_ns(9,223,372,036,854,775,807) ?

Yes, or you can find them in the built in table function test_all_types().

I've added a test for the maximum values.

For negative values, I've ran into some problems:
make_timestamp_ns(-1) triggers an assertion:

select make_timestamp_ns(-1); INTERNAL Error: Information loss on integer cast: value -952 outside of target range [-128, 127]

I couldn't immediately figure out why, or how to get a stack trace for the respective exception.

select make_timestamp(-9223372036854775807) correctly says -infinity.

make_timestamp_ns(-9223372036854775806) says:

select make_timestamp_ns(-9223372036854775806); Conversion Error: Date out of range in timestamp_ns conversion

This is similar to make_timestamp(-9223372036854775806). I don't know if the behavior is good; I couldn't find a lower limit for make_timestamp() documented anywhere.

Would it be acceptable to reject any negative argument to make_timestamp_ns() ? I'll kindly take advice for how to proceed / if you could look into the assertion error if rejecting negative input is not acceptable, I'd appreciate it.

MakeTimestampOperator was claiming to be generic in the type of timestamp it can produce, but in fact it was only producing timestamp_t. Make it actually generic; in the next patch we'll use it to produce timestamp_ns_t.

Add a function constructing a TIMESTAMP_NS from nanos since epoch. This function is modeled after one of the overloads of make_timestamp(micros) which constructs a TIMESTAMP.

Before this patch, epoch_ns only worked on TIMESTAMP input. Calling it on a TIMESTAMP_NS implied a cast, which dropped from nanos resolution to micros. This patch adds an overload for TIMESTAMP_NS. Beyond being useful at face value, this new overload also means that you now have a way of computing the difference between two TIMESTAMP_NS values and retaining nanos precision (simply doing TIMESTAMP_NS - TIMESTAMP_NS gives you an INTERVAL, which implies micros precision).

hawkfish suggested changes Nov 20, 2024

View reviewed changes

hawkfish requested a review from taniabogatsch November 20, 2024 20:36

andreimatei force-pushed the timestamp-nanos branch from 90b87fb to 43a1d84 Compare November 20, 2024 21:18

duckdb-draftbot marked this pull request as draft November 20, 2024 21:19

andreimatei force-pushed the timestamp-nanos branch from 43a1d84 to da42e2d Compare November 20, 2024 23:22

andreimatei marked this pull request as ready for review November 21, 2024 21:10

taniabogatsch approved these changes Nov 22, 2024

View reviewed changes

taniabogatsch reviewed Nov 22, 2024

View reviewed changes

hawkfish approved these changes Nov 22, 2024

View reviewed changes

andreimatei added 5 commits November 25, 2024 11:07

fix typo in readme

1819d4c

add comment to timestamp_t

5b5b074

minor typing fix

ab220f8

MakeTimestampOperator was claiming to be generic in the type of timestamp it can produce, but in fact it was only producing timestamp_t. Make it actually generic; in the next patch we'll use it to produce timestamp_ns_t.

add a make_timestamp_ns(nanos) function

c7e6070

Add a function constructing a TIMESTAMP_NS from nanos since epoch. This function is modeled after one of the overloads of make_timestamp(micros) which constructs a TIMESTAMP.

andreimatei force-pushed the timestamp-nanos branch from da42e2d to e7a2a18 Compare November 25, 2024 16:31

duckdb-draftbot marked this pull request as draft November 25, 2024 17:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add core functions make_timestamp_ns(nanos) and epoch_ns(timestamp_ts) #14930

add core functions make_timestamp_ns(nanos) and epoch_ns(timestamp_ts) #14930

add core functions make_timestamp_ns(nanos) and epoch_ns(timestamp_ts) #14930

Are you sure you want to change the base?

add core functions make_timestamp_ns(nanos) and epoch_ns(timestamp_ts) #14930

Conversation

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment