ZhangHuiGui commented on code in PR #44057:
URL: https://github.com/apache/arrow/pull/44057#discussion_r1772582207
##########
cpp/src/arrow/vendored/datetime/date.h:
##########
@@ -745,6 +750,65 @@ template<class CharT, class Traits>
std::basic_ostream<CharT, Traits>&
operator<<(std::basic_ostream<CharT, Traits>& os, const year_month_day& ymd);
+// class year_month_day_pg
+
+class year_month_day_pg {
Review Comment:
Is there a way to simplify this class? The difference from the original
`year_month_day` is mainly in `from_days` and `to_days`. Year_month_day can be
used as a class template. We can specialize different types of implementations.
PostgreSQL's `to_days/from_days` is one of the specializations, and other
functions can use the default ones.
Otherwise, there will be a lot of code redundancy when we need to be
compatible with the temporal functions of other databases/data management
systems in the future.
##########
cpp/src/arrow/compute/api_scalar.cc:
##########
@@ -864,6 +864,9 @@ SCALAR_EAGER_UNARY(Millisecond, "millisecond")
SCALAR_EAGER_UNARY(Minute, "minute")
SCALAR_EAGER_UNARY(Month, "month")
SCALAR_EAGER_UNARY(Nanosecond, "nanosecond")
+SCALAR_EAGER_UNARY(PGDay, "pg_day")
+SCALAR_EAGER_UNARY(PGMonth, "pg_month")
+SCALAR_EAGER_UNARY(PGYear, "pg_year")
Review Comment:
`pg_week` can also be supported to provide the same capabilities as
`ISOWeek` and `USWeek`.
##########
cpp/src/arrow/compute/api_scalar.h:
##########
@@ -1332,6 +1332,40 @@ Result<Datum> ISOYear(const Datum& values, ExecContext*
ctx = NULLPTR);
ARROW_EXPORT
Result<Datum> USYear(const Datum& values, ExecContext* ctx = NULLPTR);
+/// \brief Year returns PostgreSQL year for each element of `values`
Review Comment:
Here can be a more detailed description of `PGYear` and other types of Year,
where are the main differences.
##########
cpp/src/arrow/vendored/datetime/date.h:
##########
@@ -89,6 +89,11 @@ namespace arrow_vendored
namespace date
{
+// 10957 = julian days of '2000-01-01' - julian days of '1970-01-01'
+// This is used to align the number of days based on the PG epoch to
Review Comment:
It is best to use the full name of `PostgreSQL` here rather than `PG`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
