wypoon commented on code in PR #12117:
URL: https://github.com/apache/iceberg/pull/12117#discussion_r1936720348


##########
spark/v3.5/spark/src/main/java/org/apache/iceberg/spark/functions/SparkFunctions.java:
##########
@@ -29,24 +29,41 @@ public class SparkFunctions {
 
   private SparkFunctions() {}
 
+  private static final UnboundFunction VERSION_FUNCTION = new 
IcebergVersionFunction();
+  private static final UnboundFunction YEAR_FUNCTION = new YearsFunction(true);
+  private static final UnboundFunction YEARS_FUNCTION = new YearsFunction();
+  private static final UnboundFunction MONTH_FUNCTION = new 
MonthsFunction(true);
+  private static final UnboundFunction MONTHS_FUNCTION = new MonthsFunction();
+  private static final UnboundFunction DAY_FUNCTION = new DaysFunction(true);
+  private static final UnboundFunction DAYS_FUNCTION = new DaysFunction();
+  private static final UnboundFunction HOUR_FUNCTION = new HoursFunction(true);
+  private static final UnboundFunction HOURS_FUNCTION = new HoursFunction();
+  private static final UnboundFunction BUCKET_FUNCTION = new BucketFunction();
+  private static final UnboundFunction TRUNCATE_FUNCTION = new 
TruncateFunction();
+
   private static final Map<String, UnboundFunction> FUNCTIONS =
-      ImmutableMap.of(
-          "iceberg_version", new IcebergVersionFunction(),
-          "years", new YearsFunction(),
-          "months", new MonthsFunction(),
-          "days", new DaysFunction(),
-          "hours", new HoursFunction(),
-          "bucket", new BucketFunction(),
-          "truncate", new TruncateFunction());
+      new ImmutableMap.Builder<String, UnboundFunction>()
+          .put("iceberg_version", VERSION_FUNCTION)
+          .put("year", YEAR_FUNCTION)
+          .put("years", YEARS_FUNCTION)
+          .put("month", MONTH_FUNCTION)
+          .put("months", MONTHS_FUNCTION)
+          .put("day", DAY_FUNCTION)
+          .put("days", DAYS_FUNCTION)
+          .put("hour", HOUR_FUNCTION)
+          .put("hours", HOURS_FUNCTION)
+          .put("bucket", BUCKET_FUNCTION)
+          .put("truncate", TRUNCATE_FUNCTION)
+          .build();
 
   private static final Map<Class<?>, UnboundFunction> CLASS_TO_FUNCTIONS =
       ImmutableMap.of(
-          YearsFunction.class, new YearsFunction(),
-          MonthsFunction.class, new MonthsFunction(),
-          DaysFunction.class, new DaysFunction(),
-          HoursFunction.class, new HoursFunction(),
-          BucketFunction.class, new BucketFunction(),
-          TruncateFunction.class, new TruncateFunction());
+          YearsFunction.class, YEARS_FUNCTION,
+          MonthsFunction.class, MONTHS_FUNCTION,
+          DaysFunction.class, DAYS_FUNCTION,
+          HoursFunction.class, HOURS_FUNCTION,
+          BucketFunction.class, BUCKET_FUNCTION,
+          TruncateFunction.class, TRUNCATE_FUNCTION);

Review Comment:
   I had to make a choice here. `CLASS_TO_FUNCTIONS` is used by 
`loadFunctionsByClass(Class)` and that is called by the `ReplaceStaticInvoke` 
rule. The `StaticInvoke` contains a class and that is all we have to go by; we 
do not know if the class belongs to an instance with `singular` equal to true 
or false. So I map the classes to function instances of the plural form (to 
preserve existing behavior).
   I didn't feel it worthwhile to have two classes instead of one to 
distinguish between singular and plural forms.



-- 
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: issues-unsubscr...@iceberg.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@iceberg.apache.org
For additional commands, e-mail: issues-h...@iceberg.apache.org

Reply via email to