Skip to content

feat: add java.sql.Date converters (#1017) - #1289

Open
Mikkey-f wants to merge 1 commit into
apache:mainfrom
Mikkey-f:feat/sqldate-converters
Open

Mikkey-f wants to merge 1 commit into
apache:mainfrom
Mikkey-f:feat/sqldate-converters

Conversation

@Mikkey-f

@Mikkey-f Mikkey-f commented Nov 1, 2026

Copy link
Copy Markdown
Contributor

Purpose of the pull request

Related: #1017 (java.sql.Date slice). Proposal: #1017 (comment)

java.sql.Date is the last java.sql type without converters. Converters are keyed by the exact class, so a java.sql.Date field fails on read and write with Can not find 'Converter' support class Date, even though java.util.Date is supported. This mirrors java.sql.Time (#1092) and java.sql.Timestamp (#1220).

What's changed?

Added converters/sqldate and registered it in DefaultConverterLoader:

  • SqlDateNumberConverter (Excel NUMBER): reads through DateUtils.getLocalDate(serial, use1904windowing) → Date.valueOf(LocalDate), returning null for invalid serials; writes DateUtil.getExcelDate(value.toLocalDate(), use1904windowing).
  • SqlDateStringConverter (Excel STRING): parses/formats through LocalDate, honoring @DateTimeFormat and GlobalConfiguration.locale.
  • SqlDateDateConverter (write only, DATE cell): uses DateUtils.defaultLocalDateFormat unless @DateTimeFormat overrides it.

Registration matches the java.sql.Time / java.sql.Timestamp pattern; no existing behavior changes.

Verification

  • Red side (registrations removed): the XLSX/XLS/CSV round trip fails on all three formats with ExcelWriteDataConvertException: Can not find 'Converter' support class Date, and the loader registration test fails.
  • Green side: SqlDateConverterTest 10/10 and SqlDateRoundTripTest 3/3 (XLSX, XLS, CSV).
  • Full build mvn clean package -B -Dmaven.test.skip=false -pl fesod-common,fesod-shaded,fesod-sheet: 1,862 tests, 0 failures.
  • spotless:check and javadoc:javadoc pass.

Checklist

  • I have read the Contributor Guide.
  • I have written the necessary doc or comment.
  • I have added the necessary unit tests and all cases have passed.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant