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