You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@fesod.apache.org by "Anubyss-mihail (via GitHub)" <gi...@apache.org> on 2026/09/08 08:23:26 UTC
[PR] feat: add java.sql.Date converters (#1017) [fesod]
Anubyss-mihail opened a new pull request, #1079:
URL: https://github.com/apache/fesod/pull/1079
## Purpose of the pull request
Related: #1017
Add support for `java.sql.Date` conversion.
## What's changed?
* Added converters for DATE, NUMBER, and STRING cell types.
* Registered the new converters in `DefaultConverterLoader`.
* Added unit tests for the new converters.
## Checklist
* [x] I have read the [[Contributor Guide](https://fesod.apache.org/community/contribution/)](https://fesod.apache.org/community/contribution/).
* [x] I have written the necessary doc or comment.
* [x] I have added the necessary unit tests and all cases have passed.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on PR #1079:
URL: https://github.com/apache/fesod/pull/1079#issuecomment-5650309364
The conversion logic of `SqlDateDateConverter` and the date offset logic of `use1904Windowing` belong to two independent paths, and both affect the final date parsing result. Therefore, both scenarios need to be covered by tests.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #1079:
URL: https://github.com/apache/fesod/pull/1079#discussion_r3998381228
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/sqldate/SqlDateNumberConverter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.converters.sqldate;
+
+import java.math.BigDecimal;
+import java.sql.Date;
+import org.apache.fesod.sheet.converters.Converter;
+import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.metadata.GlobalConfiguration;
+import org.apache.fesod.sheet.metadata.data.ReadCellData;
+import org.apache.fesod.sheet.metadata.data.WriteCellData;
+import org.apache.fesod.sheet.metadata.property.ExcelContentProperty;
+import org.apache.fesod.sheet.util.DateUtils;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+/**
+ * Date and number converter
+ *
+ *
Review Comment:
Several other files have similar Javadoc issues as well.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "Anubyss-mihail (via GitHub)" <gi...@apache.org>.
Anubyss-mihail commented on code in PR #1079:
URL: https://github.com/apache/fesod/pull/1079#discussion_r3998695605
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/sqldate/SqlDateNumberConverter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.converters.sqldate;
+
+import java.math.BigDecimal;
+import java.sql.Date;
+import org.apache.fesod.sheet.converters.Converter;
+import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.metadata.GlobalConfiguration;
+import org.apache.fesod.sheet.metadata.data.ReadCellData;
+import org.apache.fesod.sheet.metadata.data.WriteCellData;
+import org.apache.fesod.sheet.metadata.property.ExcelContentProperty;
+import org.apache.fesod.sheet.util.DateUtils;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+/**
+ * Date and number converter
+ *
+ *
Review Comment:
Thanks for the review. I’ve updated the Javadocs and added test coverage for the SQL Date converter and 1904 windowing behavior. The changes are included in the latest commit.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "bengbengbalabalabeng (via GitHub)" <gi...@apache.org>.
bengbengbalabalabeng commented on code in PR #1079:
URL: https://github.com/apache/fesod/pull/1079#discussion_r3979713657
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/sqldate/SqlDateNumberConverter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.converters.sqldate;
+
+import java.math.BigDecimal;
+import java.sql.Date;
+import org.apache.fesod.sheet.converters.Converter;
+import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.metadata.GlobalConfiguration;
+import org.apache.fesod.sheet.metadata.data.ReadCellData;
+import org.apache.fesod.sheet.metadata.data.WriteCellData;
+import org.apache.fesod.sheet.metadata.property.ExcelContentProperty;
+import org.apache.fesod.sheet.util.DateUtils;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+/**
+ * Date and number converter
+ *
+ *
Review Comment:
The current Javadoc (“Date and number converter”) doesn’t fully match the actual implementation, which is specifically a `java.sql.Date` converter.
Could you update the Javadoc, and also remove the unused blank comment lines?
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "nkuprins (via GitHub)" <gi...@apache.org>.
nkuprins commented on code in PR #1079:
URL: https://github.com/apache/fesod/pull/1079#discussion_r4025229237
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/sqldate/SqlDateNumberConverter.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.converters.sqldate;
+
+import java.math.BigDecimal;
+import java.sql.Date;
+import org.apache.fesod.sheet.converters.Converter;
+import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.metadata.GlobalConfiguration;
+import org.apache.fesod.sheet.metadata.data.ReadCellData;
+import org.apache.fesod.sheet.metadata.data.WriteCellData;
+import org.apache.fesod.sheet.metadata.property.ExcelContentProperty;
+import org.apache.fesod.sheet.util.DateUtils;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+/**
+ * Converter for {@link java.sql.Date} and Excel number cells.
+ */
+public class SqlDateNumberConverter implements Converter<Date> {
+
+ @Override
+ public Class<?> supportJavaTypeKey() {
+ return Date.class;
+ }
+
+ @Override
+ public CellDataTypeEnum supportExcelTypeKey() {
+ return CellDataTypeEnum.NUMBER;
+ }
+
+ @Override
+ public Date convertToJavaData(
+ ReadCellData<?> cellData, ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) {
+ java.util.Date date = DateUtils.getJavaDate(
+ cellData.getNumberValue().doubleValue(), DateUtils.isDate1904(contentProperty, globalConfiguration));
+ return new Date(date.getTime());
Review Comment:
`java.sql.Date` contract requires normalised millis. `getJavaDate` also returns `null`.
Should we replace it with the following?
```java
LocalDate localDate = DateUtils.getLocalDate(
cellData.getNumberValue().doubleValue(), DateUtils.isDate1904(contentProperty, globalConfiguration));
return localDate == null ? null : Date.valueOf(localDate);
```
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "Anubyss-mihail (via GitHub)" <gi...@apache.org>.
Anubyss-mihail closed pull request #1079: feat: add java.sql.Date converters (#1017)
URL: https://github.com/apache/fesod/pull/1079
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "Anubyss-mihail (via GitHub)" <gi...@apache.org>.
Anubyss-mihail commented on code in PR #1079:
URL: https://github.com/apache/fesod/pull/1079#discussion_r3995450337
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/sqldate/SqlDateNumberConverter.java:
##########
@@ -0,0 +1,64 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.converters.sqldate;
+
+import java.math.BigDecimal;
+import java.sql.Date;
+import org.apache.fesod.sheet.converters.Converter;
+import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.metadata.GlobalConfiguration;
+import org.apache.fesod.sheet.metadata.data.ReadCellData;
+import org.apache.fesod.sheet.metadata.data.WriteCellData;
+import org.apache.fesod.sheet.metadata.property.ExcelContentProperty;
+import org.apache.fesod.sheet.util.DateUtils;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+/**
+ * Date and number converter
+ *
+ *
Review Comment:
Thanks! I've updated the Javadoc to clarify that this converter is specifically for java.sql.Date and removed the unnecessary blank comment lines.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org
Re: [PR] feat: add java.sql.Date converters (#1017) [fesod]
Posted by "Anubyss-mihail (via GitHub)" <gi...@apache.org>.
Anubyss-mihail commented on code in PR #1079:
URL: https://github.com/apache/fesod/pull/1079#discussion_r4096790883
##########
fesod-sheet/src/main/java/org/apache/fesod/sheet/converters/sqldate/SqlDateNumberConverter.java:
##########
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.fesod.sheet.converters.sqldate;
+
+import java.math.BigDecimal;
+import java.sql.Date;
+import org.apache.fesod.sheet.converters.Converter;
+import org.apache.fesod.sheet.enums.CellDataTypeEnum;
+import org.apache.fesod.sheet.metadata.GlobalConfiguration;
+import org.apache.fesod.sheet.metadata.data.ReadCellData;
+import org.apache.fesod.sheet.metadata.data.WriteCellData;
+import org.apache.fesod.sheet.metadata.property.ExcelContentProperty;
+import org.apache.fesod.sheet.util.DateUtils;
+import org.apache.poi.ss.usermodel.DateUtil;
+
+/**
+ * Converter for {@link java.sql.Date} and Excel number cells.
+ */
+public class SqlDateNumberConverter implements Converter<Date> {
+
+ @Override
+ public Class<?> supportJavaTypeKey() {
+ return Date.class;
+ }
+
+ @Override
+ public CellDataTypeEnum supportExcelTypeKey() {
+ return CellDataTypeEnum.NUMBER;
+ }
+
+ @Override
+ public Date convertToJavaData(
+ ReadCellData<?> cellData, ExcelContentProperty contentProperty, GlobalConfiguration globalConfiguration) {
+ java.util.Date date = DateUtils.getJavaDate(
+ cellData.getNumberValue().doubleValue(), DateUtils.isDate1904(contentProperty, globalConfiguration));
+ return new Date(date.getTime());
Review Comment:
Thanks for the suggestion. I've updated the implementation to use DateUtils.getLocalDate() and handle the null case.
--
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: commits-unsubscribe@fesod.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@fesod.apache.org
For additional commands, e-mail: commits-help@fesod.apache.org