Velox doesn't support legacy date format behavior in Spark SQL

[NEUpanning]

Bug description

For backward capability of parsing/formatting of timestamp/date strings expression behavior and adoption of new behaviors. Spark adds a setting spark.sql.legacy.timeParserPolicy to indicate whether using legacy behavior. In legacy behavior, Spark does parse/format date using SimpleDateFormat that has different standard and behavior with Velox Joda. For example, SimpleDateFormat doesn't performs strict checking for expression's input and Velox doesn't support this when using unix_timestamp function, there is a related gluten issue : link. There are more result mismatch cases include gluten#7109 gluten#7069

The main difference between Spark SimpleDateFormat and Velox Joda

SimpleDateFormat Letter vs Joda (Velox) Letter

Field (SimpleDateFormat)	Field (Joda)	Desc	Velox Support
	C	century of era (>=0)
F		Day of week in month	Unsupported
L	M	Month in year (standalone form)
M		Month in year (context sensitive)	Unsupported
W		Week in month	#10511
	Y	year of era (>=0)
Y	x	Week year	Unsupported
Z	Z	Time zone	Unsupported
u	e	day of week

Range of field value

SimpleDateFormat in non-lenient mode:

/*
     * <pre>
     *                            Greatest       Least
     * Field name        Minimum   Minimum     Maximum     Maximum
     * ----------        -------   -------     -------     -------
     * ERA                     0         0           1           1
     * YEAR                    1         1   292269054   292278994
     * MONTH                   0         0          11          11
     * WEEK_OF_YEAR            1         1          52*         53
     * WEEK_OF_MONTH           0         0           4*          6
     * DAY_OF_MONTH            1         1          28*         31
     * DAY_OF_YEAR             1         1         365*        366
     * DAY_OF_WEEK             1         1           7           7
     * DAY_OF_WEEK_IN_MONTH    1         1           4*          6
     * AM_PM                   0         0           1           1
     * HOUR                    0         0          11          11
     * HOUR_OF_DAY             0         0          23          23
     * MINUTE                  0         0          59          59
     * SECOND                  0         0          59          59
     * MILLISECOND             0         0         999         999
     * ZONE_OFFSET        -13:00    -13:00       14:00       14:00
     * DST_OFFSET           0:00      0:00        0:20        2:00
     * </pre>
     * *: depends on the Gregorian change date
     */

Behavior for invalid format

select from_unixtime(time, "AA") from test
spark result:null
gluten result:Error

Calendar supporting

SimpleDateFormat supports both the Julian and Gregorian calendar systems with the support of a single discontinuity. But Velox Joda only supports Gregorian Calendar.

Behavior when not cosuming entire string

spark result : result value
gluten result : Error
gluten issue

Obey the number of pattern letters for parsing

SimpleDateFormat obeys the number of pattern letters for parsing when the next pattern is numeric field, but Velox always obeys the number of pattern letters.

Road map

• Support new DateTimeFormatterType called 'LENIENT_SIMPLE' and 'STRICT_SIMPLE' for Java's java.text.SimpleDateFormat in lenient and non-lenient mode Introduce simple date time formatter #10966
• Using new DateTimeFormatter in gluten if spark.sql.legacy.timeParserPolicy is LEGACY gluten#7375
• Remove specifier 'C'
• Remove specifier 'Y'
• Support specifier 'F'
• Support specifier 'L'
• Support specifier 'W', which represents 'week of month'. Add support for "week of month" in SimpleDateTimeFormatter #11103 Add helper function to return the number of days since epoch for a week-of-month date #10604
• Support specifier 'M'
• Support specifier 'Y', which represents 'week year'. Support WEEK_YEAR for date time formatter #10930
• Support specifier 'Z'
• Support specifier 'u'
• Support specifier 'w'
• Support both the Julian and Gregorian calendar systems, with the cutover date being October 15, 1582.
• Fix date range validation in SimpleDateFormat standard
• Return a result for not using the entire text of the given string rather than throwing an exception when parsing the date. Allow partial date parsing when simple datetime formatter is used #11386
• Support the behavior of Spark for invalid format Return null for invalid datetime format when legacy date formatter is used #11131
• Support the behavior of obeying the number of pattern letters for parsing

[NEUpanning]

Maybe we should add support for legacy date format behavior. @pedroerp @rui-mo What do you think? Thanks!

[PHILO-HE]

@NEUpanning, is Spark's legacy policy widely used in your production environment?

[NEUpanning]

@PHILO-HE Yes. In Spark upgrades, we need to use legacy policy for backward compatibility rather than pushing users to accept the new behavior. Here is part of legacy policies we are using :

spark.sql.storeAssignmentPolicy Legacy
spark.sql.legacy.createArrayOneBasedIndex false
spark.sql.legacy.setCommandRejectsSparkCoreConfs false
spark.sql.legacy.ctePrecedencePolicy LEGACY
spark.sql.legacy.createEmptyCollectionUsingStringType true
spark.sql.legacy.exponentLiteralAsDecimal.enabled true
spark.sql.legacy.parser.havingWithoutGroupByAsWhere true
spark.sql.legacy.timeParserPolicy LEGACY
spark.sql.legacy.dataset.nameNonStructGroupingKeyAsValue true
spark.sql.legacy.fromDayTimeString.enabled true
spark.sql.legacy.allowHashOnMapType true
spark.sql.legacy.allowNegativeScaleOfDecimal true
spark.sql.legacy.followThreeValuedLogicInArrayExists false
spark.sql.legacy.json.allowEmptyString.enabled true
spark.sql.legacy.setopsPrecedence.enabled true
spark.sql.legacy.literal.pickMinimumPrecision false
spark.sql.legacy.sizeOfNull true
spark.sql.legacy.typeCoercion.datetimeToString.enabled true
spark.sql.legacy.doLooseUpcast true
spark.sql.legacy.addMonthsAdjustLastDay true
spark.sql.legacy.statisticalAggregate true

[pedroerp]

@NEUpanning: we have recently added support for different date/timestamp parsing semantics; are these not enough to follow the Spark parsing semantic you described? If not, could you highlight some of the differences?

https://github.com/facebookincubator/velox/blob/main/velox/type/TimestampConversion.h#L48

Cc: @mbasmanova

[NEUpanning]

@pedroerp Thanks for your reply. UnixTimestampParseFunction is using DateTimeFormatter to parse date : source code link. DateTimeFormatter doesn't support different date/timestamp parsing semantics like TimestampConversion.

[NEUpanning]

@kecookier

[pedroerp]

@NEUpanning DateTimeFormatter has different parsing "types". My point was that, even if none of these types support the exact parsing semantics you need today, they could be extended to do so:

https://github.com/facebookincubator/velox/blob/main/velox/functions/lib/DateTimeFormatter.h#L26

[NEUpanning]

@pedroerp I'd like to add a new DateTimeFormatterType called SIMPLE for Java's java.text.SimpleDateFormat to support legacy date parsing behavior in unix_timestamp function. And i think we need to add a flag to indicate which DateTimeFormatterType to use in Spark unix_timestamp function. How do you think?

[pedroerp]

Sounds reasonable. Do you have any examples on how the parsing compares with Joda parsing?

Cc: @mbasmanova who recently enhanced this part of the code.

[NEUpanning]

@pedroerp Here is an example I wrote from scratch.

code :

import java.text.ParseException;
import java.text.SimpleDateFormat;
import org.joda.time.DateTime;
import org.joda.time.format.DateTimeFormat;
import org.joda.time.format.DateTimeFormatter;import java.util.Date;

public class Test {
    static SimpleDateFormat simpleDateFormat = new SimpleDateFormat("yyyy-MM-dd");
    static DateTimeFormatter jodaFormatter = DateTimeFormat.forPattern("yyyy-MM-dd");
    public static void invalidMonth() {
        // Invalid month (13)
        String dateString = "2023-13-01";

        // SimpleDateFormat can parse the invalid date
        try {
            Date date = simpleDateFormat.parse(dateString);
            System.out.println("SimpleDateFormat parsed date: " + date);
        } catch (ParseException e) {
            System.out.println("SimpleDateFormat: " + e.getMessage());
        }

        // Joda-Time will throw exception
        try {
            DateTime dateTime = jodaFormatter.parseDateTime(dateString);
            System.out.println("Joda-Time parsed date: " + dateTime);
        } catch (IllegalArgumentException e) {
            System.out.println("Joda-Time: " + e.getMessage());
        }
    }

    public static void dateWithExtraCharacters() {
        // Date with extra Characters
        String dateString = "2023-12-01 00:00:00";

        // SimpleDateFormat does not use all characters up to the end of the string
        try {
            Date date = simpleDateFormat.parse(dateString);
            System.out.println("SimpleDateFormat parsed date: " + date);
        } catch (ParseException e) {
            System.out.println("SimpleDateFormat: " + e.getMessage());
        }

        // Joda-Time consumes all characters
        DateTimeFormatter jodaFormatter = DateTimeFormat.forPattern("yyyy-MM-dd");
        try {
            DateTime dateTime = jodaFormatter.parseDateTime(dateString);
            System.out.println("Joda-Time parsed date: " + dateTime);
        } catch (IllegalArgumentException e) {
            System.out.println("Joda-Time: " + e.getMessage());
        }
    }

    public static void main(String[] args) throws ParseException {
        invalidMonth();
        dateWithExtraCharacters();
    }
}

output :

SimpleDateFormat parsed date: Mon Jan 01 00:00:00 CST 2024
Joda-Time: Cannot parse "2023-13-01": Value 13 for monthOfYear must be in the range [1,12]
SimpleDateFormat parsed date: Fri Dec 01 00:00:00 CST 2023
Joda-Time: Invalid format: "2023-12-01 00:00:00" is malformed at " 00:00:00"

[NEUpanning]

@pedroerp I'm implementing new DateTimeFormatterType and i see DateTimeFormatSpecifier doesn't support Week in month pattern used by java.text.SimpleDateFormat. Should i implement it in another PR or in the PR that addresses this issue?

[mbasmanova]

Should i implement it in another PR

@NEUpanning A separate PR would be nice. Smaller PRs are easier to review. Thanks.

[ccat3z]

Sounds reasonable. Do you have any examples on how the parsing compares with Joda parsing?

Cc: @mbasmanova who recently enhanced this part of the code.

Here is the difference between joda and SimpleDateFormat:

Field (SimpleDateFormat)	Field (Joda)	Desc	Velox Support
	C	century of era (>=0)
F		Day of week in month	Unsupported
L	M	Month in year (standalone form)
M		Month in year (context sensitive)	Unsupported
W		Week in month	#10511
	Y	year of era (>=0)
Y	x	Week year	Unsupported
Z	Z	Time zone	Unsupported
u	e	day of week

@pedroerp I'd like to add a new DateTimeFormatterType called SIMPLE for Java's java.text.SimpleDateFormat to support legacy date parsing behavior in unix_timestamp function. And i think we need to add a flag to indicate which DateTimeFormatterType to use in Spark unix_timestamp function. How do you think?

I'm in favor of this approach too, because there are conflicting parts between the two formats. cc @rui-mo, @NEUpanning

[rui-mo]

I'd like to add a new DateTimeFormatterType called SIMPLE for Java's java.text.SimpleDateFormat

@ccat3z Thanks for providing the details. Sounds good to me generally. What are the functions that are impacted by this date format issue, if you could list them?

[ccat3z]

I'd like to add a new DateTimeFormatterType called SIMPLE for Java's java.text.SimpleDateFormat

@ccat3z Thanks for providing the details. Sounds good to me generally. What are the functions that are impacted by this date format issue, if you could list them?

from_unixtime, to_unix_timestamp

[NEUpanning]

We encountered more result mismatch issues due to Velox joda Dateformatter not aligning with Spark SimpleDateFormat. I did some research on Spark SimpleDateFormat. And I updated the issue description to include the main difference between Spark SimpleDateFormat and Velox Joda and the tasks to resolve this issue.
cc @rui-mo @mbasmanova @kecookier @pedroerp