On Mon, 8 Jul 2024 at 14:50, jian he <jian.universal...@gmail.com> wrote: > > looks good to me.
Thanks for looking. > /* > * Protect against overflows in timestamp_mi. XXX convert to > * ereturn one day? > */ > if (!TIMESTAMP_NOT_FINITE(start) && !TIMESTAMP_NOT_FINITE(finish) && > !pg_sub_s64_overflow(finish, start, &dummy)) > > i don't understand the comment "XXX convert to ereturn one day?". The problem I'm trying to work around there is that timestamp_mi raises an ERROR if there's an overflow. I don't want the support function to cause an ERROR so I'm trying to only call timestamp_mi in cases where it won't error. The ereturn mention is a reference to ERROR raising infrastructure added by d9f7f5d32 and so far only used by input functions. It would be possible to use that to save from having to do the pg_sub_s64_overflow(). Instead, we could check if any errors were found and only proceed with the remaining part of the calculation if none were found. I've tried to improve the comment in the attached version. I removed the reference to ereturn. > do we need to add unlikely for "pg_sub_s64_overflow", i saw most of > pg_sub_s64_overflow have unlikely. I was hoping the condition would be likely() rather than unlikely(). However, I didn't consider that the code path was hot enough for it to matter. It's just a function we call once during planning if we find a call to generate_series_timestamp(). It's not like it's called a million or a billion times during execution like a function such as int4pl() could be. David
v3-0001-Add-support-function-for-generate_series-for-time.patch
Description: Binary data
