-
Notifications
You must be signed in to change notification settings - Fork 73
add SXSS writing #1075
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
add SXSS writing #1075
Conversation
WorkBookType.XLSX -> XSSFWorkbook(file.inputStream()) | ||
// Write to an existing file with `keepFile` flag | ||
if (keepFile && file.exists() && file.length() > 0L) { | ||
file.inputStream().use { fis -> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please check if it's ok to use use
here, it will close input stream right after workbook is created
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIU Apache POI loads data from the InputStream into memory when the Workbook is created. I tested it with keepFile=true
and it works as expected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw looks like using fis increases memory:
Opening a XSSFWorkbook from a file has a lower memory footprint than opening from an InputStream
May be use file directly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's do it, sure :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It doesn't work 🤣 🤣 🤣 . Known APOI problem.
Worth to mention that keepFile = true will lead to increased memory usage due to XSSFWorkbook compared to false (default) |
Added KDocs for |
* @param sheetName The name of the sheet in the Excel file. If null, the default name will be used. | ||
* @param writeHeader A flag indicating whether to write the header row in the Excel file. Defaults to true. | ||
* @param workBookType The [type of workbook][WorkBookType] to create (e.g., XLS or XLSX). Defaults to XLSX. | ||
* @param keepFile If `true` and the file already exists, a new sheet will be appended instead of overwriting the file. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if sheetName already exists in the workbook, will it be overwritten? maybe comment needs to be adjusted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Throws:
IllegalArgumentException – if the name is null or invalid or workbook already contains a sheet with this name
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the best thing to write in our doc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm.. maybe @throws
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you've tested all mentioned edge cases, it looks like a nice drop-in replacement
Fixes #1016.
Tested with profiler on big datasets - works great :).
Looks like
SXSSFWorkbook
restrictions don't affect on DF writing into Excel (except for the case for writing into existing file - in this case it can't just be used andXSSFWorkbook
is used).