Skip to content

feat: add java.sql.Clob converter - #1134

Open
AhmedYoussef98 wants to merge 1 commit into
apache:mainfrom
AhmedYoussef98:feat-clob-converter
Open

AhmedYoussef98 wants to merge 1 commit into
apache:mainfrom
AhmedYoussef98:feat-clob-converter

Conversation

@AhmedYoussef98

Copy link
Copy Markdown

Purpose of the pull request

Related to #1017

What's changed?

Adds a default java.sql.Clob converter. Clob values are written to Excel as string cells by reading their character stream. String cells are converted back to detached SerialClob instances, avoiding database-specific implementations.

The converter is registered for default reads and writes. Unit tests cover converter keys, both conversion directions, and loader registration.

Verification

  • ./mvnw package -B -Dmaven.test.skip=false -Dtest=ClobStringConverterTest,DefaultConverterLoaderTest -Dsurefire.failIfNoSpecifiedTests=false -pl fesod-common,fesod-shaded,fesod-sheet — 8 tests passed.
  • ./mvnw spotless:check -B — passed.

The full spreadsheet suite did not complete on macOS because CompatibilityTest.readWithCacheAfterTempDeletion recursed repeatedly through FileUtils.delete; the targeted converter and loader tests passed.

Checklist

  • I have read the Contributor Guide.
  • I have added the necessary unit tests.

@skytin1004 skytin1004 left a comment •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this. I verified that the default converter handles a SerialClob correctly in the standard XLSX, XLS, and CSV read/write paths, and that the character stream is properly closed. All tests passed locally on Java 11. LGTM.

}

@Test
void convertsClobCharacterStreamToCellString() throws Exception {

@nkuprins nkuprins Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Is there a test gap on the write path? convertsClobCharacterStreamToCellString uses a 13-char value, so the 4096-char read loop only runs once. Changing while to if (so only the first chunk is kept) still passes all three tests. Using the cell text limit as the length covers several chunks and delei's boundary at the same time:

@Test
void convertsClobAtCellTextLimitToCellString() throws Exception {
    StringBuilder expected = new StringBuilder();
    for (int i = 0; i < 32767; i++) {
        expected.append((char) ('a' + i % 26));
    }
    Clob value = new SerialClob(expected.toString().toCharArray());

    WriteCellData<?> written = converter.convertToExcelData(value, null, GLOBAL_CONFIGURATION);

    Assertions.assertEquals(expected.toString(), written.getStringValue());
}

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.

3 participants