Skip to content

Conversation

@ongdisheng
Copy link
Contributor

@ongdisheng ongdisheng commented Nov 30, 2025

Closed: #714

Purpose of the pull request

This PR fixes a bug where using java.sql.Date with WriteCellData constructor throws an UnsupportedOperationException. The issue occurs because the original implementation calls toInstant() method which is not supported by java.sql.Date.

What's changed?

The WriteCellData Date constructor now uses Instant.ofEpochMilli(dateValue.getTime()) instead of dateValue.toInstant() as the getTime() method works for both java.util.Date and java.sql.Date.

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.

@ongdisheng ongdisheng marked this pull request as draft November 30, 2025 17:05
@ongdisheng ongdisheng marked this pull request as ready for review November 30, 2025 17:23
@alaahong alaahong requested a review from Copilot December 7, 2025 07:40
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes a bug where java.sql.Date instances would throw an UnsupportedOperationException when passed to the WriteCellData constructor. The fix changes the date conversion from using toInstant() (which is not supported by java.sql.Date) to using getTime() (which works for both java.util.Date and java.sql.Date).

  • Changed the WriteCellData Date constructor to use Instant.ofEpochMilli(dateValue.getTime()) instead of dateValue.toInstant()
  • Added comprehensive test coverage for java.sql.Date compatibility
  • Updated date format in test data classes from Chinese format to standard ISO format for consistency

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
fesod/src/main/java/org/apache/fesod/sheet/metadata/data/WriteCellData.java Fixed the Date constructor to handle both java.util.Date and java.sql.Date by using getTime() method instead of toInstant()
fesod/src/test/java/org/apache/fesod/sheet/celldata/CellDataWriteData.java Added sqlDate field and updated date format to ISO format for testing
fesod/src/test/java/org/apache/fesod/sheet/celldata/CellDataReadData.java Added corresponding sqlDate field for reading test data
fesod/src/test/java/org/apache/fesod/sheet/celldata/CellDataDataTest.java Added test case that creates WriteCellData with java.sql.Date to verify the fix
fesod/src/test/java/org/apache/fesod/sheet/celldata/CellDataDataListener.java Added assertion to verify java.sql.Date handling works correctly

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

setType(CellDataTypeEnum.DATE);
this.dateValue = LocalDateTime.ofInstant(dateValue.toInstant(), ZoneId.systemDefault());
// Use getTime() which works for both java.util.Date and java.sql.Date
this.dateValue = LocalDateTime.ofInstant(Instant.ofEpochMilli(dateValue.getTime()), ZoneId.systemDefault());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using the Date#getTime() incurs a loss of precision, as it only provides millisecond accuracy. Furthermore, might we consider supporting java.sql.Timestamp? I believe the unit test examples could be improved.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the feedback! I'll look into this and explore supporting java.sql.Timestamp as well. Will improve the test examples too.

@ongdisheng ongdisheng marked this pull request as draft December 14, 2025 11:35
@ongdisheng ongdisheng changed the title fix: Handle java.sql.Date compatibility in WriteCellData fix: Handle java.sql date types compatibility in Excel read and write operations Dec 14, 2025
@ongdisheng ongdisheng marked this pull request as ready for review December 14, 2025 11:42
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.

[Bug] WriteCellData 方法如果传入 java.sql.Date 会报错

3 participants