Per https://stackoverflow.com/q/53173526/14731 (I am not the author) passing a non-null value into XSSFSheet.createTable(AreaReference) always generates a corrupt XLSX file. I ran into issue for hours until I ran across this Stackoverflow post and I can confirm that passing in null works well. Obviously I'd like to pass a non-null value into the method (as the Javadoc explicitly indicates that the parameter should not be null) but no one seems to have figured out how to make this work cleanly. Consider updating your sample code / testcases to demonstrate how this is meant to work.
I've nade a few changes in the sample code and the CreateTable xlsx is produced ok and I can open it in Mac Numbers and LibreOffice without issues. I don't have M$ producta installed on my laptop.
*** Bug 63372 has been marked as a duplicate of this bug. ***
PJ Fanning, Please post your source-code and the output file. I will test both against Microsoft Office under Windows.
Created attachment 36545 [details] Proposed fix to bug 62906 Since XSSF.createTable will corrupt a worksheet unless setDisplayName is called, this code calls setDisplayName at the end of createTable. The name that is used is "TableN", where N is the smallest positive integer such that "TableN" is not used by a previously defined name.
Comment on attachment 36545 [details] Proposed fix to bug 62906 XSSFSheet.createTable will corrupt a workbook unless CTTable.setDisplayName is called. This patch to XSSFSheet.createTable calls setDisplayName(tableName), where tableName has the text "Table<N>", where <N> is the smallest positive integer such that "Table<N>" is not used by a previously defined name.
Created attachment 36546 [details] Test case to illustrate bug 62906 This method returns an XSSFWorkbook with a single XSSFSheet containing a single XSSFTable. If it is called with bApplyBugFix set to false, the resulting workbook will be reported by Microsoft Excel 2010 as corrupt. If bApplyBugFix is set to true, the resulting workbook is opened by Microsoft Excel 2010 without problem.
Comment on attachment 36545 [details] Proposed fix to bug 62906 the tableNumber is already set by `int tableNumber = getPackagePart().getPackage().getPartsByContentType(XSSFRelation.TABLE.getContentType()).size() + 1` - any reason not to use this when setting the displayName?
That is a good idea; we can just remove the line "tableNumber=0". It is still important to test that the name does not conflict with another name. For example, one could have a non-table range named "Table1", and then add a table. It will corrupt the file if the table is named "Table1".
The CreateTable example (https://github.com/apache/poi/blob/trunk/src/examples/src/org/apache/poi/xssf/usermodel/examples/CreateTable.java) has an explicit setDisplayName - is this not enough? I'm not sure if it makes sense for us to generate this name.
The table absolutely, positively has to have a display name set or the workbook will be corrupted. It seems to me that it is best to set it to a default value, and allow the user to change that value if desired, than to let the user write code, run it, produce corrupted workbooks, and then look around to see if there is a code example that will fix the problem. Think of it as a fail-safe approach to coding. We may want to add another overload of createTable to allow the user to define a different table name instead of calling setDisplayName() after the fact.
Absolutely! David nailed it on the head. Debugging corrupt files is extremely annoying/difficult because Excel does not provide any meaningful hints of what is wrong. You should have reasonable defaults, let users override them (if so desired), and I'd even go as far as preventing users from assigning tables a null name. It should be impossible to export a corrupt file, period.
added change using https://github.com/apache/poi/commit/1b4ea32574382a784a978a94e7ecf96055ca9b17 https://builds.apache.org/view/P/view/POI/job/POI-DSL-1.8/715/artifact/build/dist/ has the change built into it
I just looked at the patch to createTable that PJ posted. It does not check to see if the table's display name conflicts with an existing name. I just tried the following in Excel 2010: 1) Create a new workbook 2) Create a defined name in the workbook "Table1" 3) Add a table to Sheet1 The table created in step (3) has the name "Table2" in order to avoid conflicting with the previously defined name "Table1". I haven't checked the patch, but I would bet that following the above procedure using POI would result in a corrupted workbook because there is named range called "Table1" and a table called "Table1". I realize that this would be a rare occurrance (who names a non-table range "Table1"?) if if we can foresee it and the prevention is simple, I would recommend the prevention.
there is existing code upping the tableNumber - so I don't want 2 blocks of code upping the number - so if you can provide a real test case that proves the existing code to up the number is wrong, then we can fix it. https://github.com/apache/poi/blob/trunk/src/ooxml/java/org/apache/poi/xssf/usermodel/XSSFSheet.java#L4116 - up to 4130
PJ Fanning, Here is a simpler testcase than what David mentioned: 1. Sheet contains "table1" (generated automatically or manually) 2. User adds another table and manually names it "table1" 3. User exports worksheet. Exported file is flagged as corrupt. Expected behavior: Setting a table to a conflicting name should throw an exception (fail fast) instead of failing much later on (e.g. when the worksheet is exported and reopened in Excel).
Another testcase: 1. User creates a table using sheet.createTable(reference). POI assigns it the name "table1". 2. User changes the table name to "table2" using setDisplayName() 3. User creates a second table using sheet.createTable(reference). POI assigns it the name "table2". 4. Worksheet is now in a bad state.
Apache POI is a volunteer project with no affiliation to Microsoft. If Microsoft Excel does not provide details about corruption in documents, then that is an issue you should bring to Microsoft.
Sure... but the fact remains that POI is generating invalid worksheets. Nicer error message or not, this is a bug.
Created attachment 36555 [details] Illustration of different kinds of table name conflicts This code can be used to demonstrate conflicts between the name of a new table and previously defined tables or previously defined Name objects. Details are included in the javadoc. In brief, the workbook will be corrupted if two Tables have the same name, or a Table has the same name as a defined Name (e.g. a named range). This is true whether the Name has workbook scope or worksheet scope, and whether the tables are in the same worksheet or different worksheets. So long as no table has a null name, Excel 2010 can repair the corruption by renaming a Table. However, there is no guarantee that this is true for other versions of Excel, or for other ooxml compliant spreadsheet programs such as OpenOffice.
Created attachment 36559 [details] Test case to illustrate bug 62906 This JUnit test class runs 3 tests: testDefaultTableName: fails if the the default name conflicts with existing XSSFTables or XSSFNames testRangeNameConflict: fails if it is possible to rename an XSSFTable using the name of an existing XSSFName testTableNameConflict: fails if it is possible to rename an XSSFTable using the name of an existing XSSFTable The test also writes xlsx files to "build/custom-reports-test". Open these files to ensure that they are not corrupted.
Created attachment 36560 [details] Patch to XSSFTable.setTableName() This patch includes a new static function, XSSFTable.isInvalidTableName, that returns true if the argument conflicts with an existing XSSFTable or XSSFName. In addition, setDisplayName will throw an InvalidArgumentException if isInvalidTableName returns true.
Created attachment 36561 [details] Patch to XSSSheet.createTable This patch calls XSSFTable.isInvalidTableName (see previous patch) before calling setDisplayName. If isInvalidTableName returns true, then tableNumber is incremented until isInvalidTableName returns false.