Skip to content
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

Improve performance of Workseet CopyTo #942

Merged
merged 5 commits into from
Dec 14, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 41 additions & 10 deletions main/HSSF/UserModel/HSSFSheet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3006,13 +3006,27 @@ public ISheet CopySheet(String Name, Boolean copyStyle)
HSSFRow destRow = (HSSFRow)newSheet.CreateRow(i);
if (srcRow != null)
{
CopyRow(this, newSheet, srcRow, destRow, styleMap, new Dictionary<short, short>(), true);
// avoid O(N^2) performance scanning through all regions for each row
// merged regions will be copied after all the rows have been copied
CopyRow(this, newSheet, srcRow, destRow, styleMap, new Dictionary<short, short>(), true, keepMergedRegions: false);
if (srcRow.LastCellNum > maxColumnNum)
{
maxColumnNum = srcRow.LastCellNum;
}
}
}

// Copying merged regions
foreach (var srcRegion in this.MergedRegions)
{
var destRegion = srcRegion.Copy();
// Additional check here as Sheet.CloneSheet() should already have copied the merged regions
if (!newSheet.IsMergedRegion(destRegion))
{
newSheet.AddMergedRegion(destRegion);
}
}

for (int i = 0; i <= maxColumnNum; i++)
{
newSheet.SetColumnWidth(i, GetColumnWidth(i));
Expand Down Expand Up @@ -3062,13 +3076,27 @@ public void CopyTo(IWorkbook dest, String name, Boolean copyStyle, Boolean keepF
HSSFRow destRow = (HSSFRow)newSheet.CreateRow(i);
if (srcRow != null)
{
CopyRow(this, newSheet, srcRow, destRow, styleMap, paletteMap, keepFormulas);
// avoid O(N^2) performance scanning through all regions for each row
// merged regions will be copied after all the rows have been copied
CopyRow(this, newSheet, srcRow, destRow, styleMap, paletteMap, keepFormulas, keepMergedRegions: false);
if (srcRow.LastCellNum > maxColumnNum)
{
maxColumnNum = srcRow.LastCellNum;
}
}
}

// Copying merged regions
foreach (var srcRegion in this.MergedRegions)
{
var destRegion = srcRegion.Copy();
// Additional check here as Sheet.CloneSheet() should already have copied the merged regions
if (!newSheet.IsMergedRegion(destRegion))
{
newSheet.AddMergedRegion(destRegion);
}
}

for (int i = 0; i < maxColumnNum; i++)
{
newSheet.SetColumnWidth(i, GetColumnWidth(i));
Expand Down Expand Up @@ -3222,7 +3250,7 @@ private static Dictionary<short,short> MergePalettes(HSSFWorkbook source, HSSFWo
}
return retval;
}
private static void CopyRow(HSSFSheet srcSheet, HSSFSheet destSheet, HSSFRow srcRow, HSSFRow destRow, IDictionary<Int32, HSSFCellStyle> styleMap, Dictionary<short, short> paletteMap, bool keepFormulas)
private static void CopyRow(HSSFSheet srcSheet, HSSFSheet destSheet, HSSFRow srcRow, HSSFRow destRow, IDictionary<Int32, HSSFCellStyle> styleMap, Dictionary<short, short> paletteMap, bool keepFormulas, bool keepMergedRegions)
{
List<SS.Util.CellRangeAddress> mergedRegions = destSheet.Sheet.MergedRecords.MergedRegions;
destRow.Height = srcRow.Height;
Expand All @@ -3243,17 +3271,20 @@ private static void CopyRow(HSSFSheet srcSheet, HSSFSheet destSheet, HSSFRow src
newCell = (HSSFCell)destRow.CreateCell(j);
}
HSSFCellUtil.CopyCell(oldCell, newCell, styleMap, paletteMap, keepFormulas);
CellRangeAddress mergedRegion = GetMergedRegion(srcSheet, srcRow.RowNum, (short)oldCell.ColumnIndex);
if (mergedRegion != null)

if (keepMergedRegions)
{
CellRangeAddress newMergedRegion = new CellRangeAddress(mergedRegion.FirstRow,
mergedRegion.LastRow, mergedRegion.FirstColumn, mergedRegion.LastColumn);
if (IsNewMergedRegion(newMergedRegion, mergedRegions))
CellRangeAddress mergedRegion = GetMergedRegion(srcSheet, srcRow.RowNum, (short)oldCell.ColumnIndex);
if (mergedRegion != null)
{
mergedRegions.Add(newMergedRegion);
CellRangeAddress newMergedRegion = new CellRangeAddress(mergedRegion.FirstRow,
mergedRegion.LastRow, mergedRegion.FirstColumn, mergedRegion.LastColumn);
if (IsNewMergedRegion(newMergedRegion, mergedRegions))
{
mergedRegions.Add(newMergedRegion);
}
}
}

}
}
}
Expand Down
43 changes: 28 additions & 15 deletions ooxml/XSSF/UserModel/XSSFSheet.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3669,10 +3669,19 @@ public void CopyTo(IWorkbook dest, string name, bool copyStyle, bool keepFormula
XSSFRow destRow = (XSSFRow)newSheet.CreateRow(i);
if (srcRow != null)
{
CopyRow(this, newSheet, srcRow, destRow, styleMap, keepFormulas);
// avoid O(N^2) performance scanning through all regions for each row
// merged regions will be copied after all the rows have been copied
CopyRow(this, newSheet, srcRow, destRow, styleMap, keepFormulas, keepMergedRegion: false);
}
}

// Copying merged regions
foreach (var srcRegion in this.MergedRegions)
{
var destRegion = srcRegion.Copy();
newSheet.AddMergedRegion(destRegion);
}

List<CT_Cols> srcCols = worksheet.GetColsList();
List<CT_Cols> dstCols = newSheet.worksheet.GetColsList();
dstCols.Clear(); //Should already be empty since this is a new sheet.
Expand Down Expand Up @@ -5627,7 +5636,7 @@ private XSSFPictureData FindPicture(IList<POIXMLDocumentPart> sheetPictures, str
return null;
}

private static void CopyRow(XSSFSheet srcSheet, XSSFSheet destSheet, XSSFRow srcRow, XSSFRow destRow, IDictionary<int, ICellStyle> styleMap, bool keepFormulas)
private static void CopyRow(XSSFSheet srcSheet, XSSFSheet destSheet, XSSFRow srcRow, XSSFRow destRow, IDictionary<int, ICellStyle> styleMap, bool keepFormulas, bool keepMergedRegion)
{
destRow.Height = srcRow.Height;
if (!srcRow.GetCTRow().IsSetCustomHeight())
Expand Down Expand Up @@ -5663,22 +5672,26 @@ private static void CopyRow(XSSFSheet srcSheet, XSSFSheet destSheet, XSSFRow src
}

CopyCell(oldCell, newCell, styleMap, keepFormulas);
CellRangeAddress mergedRegion = srcSheet.GetMergedRegion(
new CellRangeAddress(srcRow.RowNum, srcRow.RowNum,
(short)oldCell.ColumnIndex,
(short)oldCell.ColumnIndex));

if (mergedRegion != null)

if (keepMergedRegion)
{
CellRangeAddress newMergedRegion = new CellRangeAddress(
mergedRegion.FirstRow,
mergedRegion.LastRow,
mergedRegion.FirstColumn,
mergedRegion.LastColumn);
CellRangeAddress mergedRegion = srcSheet.GetMergedRegion(
new CellRangeAddress(srcRow.RowNum, srcRow.RowNum,
(short)oldCell.ColumnIndex,
(short)oldCell.ColumnIndex));

if (!destSheet.IsMergedRegion(newMergedRegion))
if (mergedRegion != null)
{
destSheet.AddMergedRegion(newMergedRegion);
CellRangeAddress newMergedRegion = new CellRangeAddress(
mergedRegion.FirstRow,
mergedRegion.LastRow,
mergedRegion.FirstColumn,
mergedRegion.LastColumn);

if (!destSheet.IsMergedRegion(newMergedRegion))
{
destSheet.AddMergedRegion(newMergedRegion);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
using System.IO;

using System.Linq;
using NPOI.HSSF.UserModel;
using NPOI.SS.UserModel;
using NPOI.SS.Util;
using NUnit.Framework;

namespace TestCases.HSSF.UserModel
{
[TestFixture]
public class TestCopySheet
public class TestHSSFSheetCopy
{
[Test]
public void TestBasicCopySheet()
Expand Down Expand Up @@ -124,5 +125,66 @@ public void TestImageCopy()
Assert.IsTrue(sanityCheck.GetAllPictures().Count == 2);
}
}

[Test]
public void CopySheetToWorkbookShouldCopyFormulasOver()
{
HSSFWorkbook srcWorkbook = new HSSFWorkbook();
HSSFSheet srcSheet = srcWorkbook.CreateSheet("Sheet1") as HSSFSheet;

// Set some values
IRow row1 = srcSheet.CreateRow((short)0);
ICell cell = row1.CreateCell((short)0);
cell.SetCellValue(1);
ICell cell2 = row1.CreateCell((short)1);
cell2.SetCellFormula("A1+1");
HSSFWorkbook destWorkbook = new HSSFWorkbook();
srcSheet.CopyTo(destWorkbook, srcSheet.SheetName, true, true);

var destSheet = destWorkbook.GetSheet("Sheet1");
Assert.NotNull(destSheet);

Assert.AreEqual(1, destSheet.GetRow(0)?.GetCell(0).NumericCellValue);
Assert.AreEqual("A1+1", destSheet.GetRow(0)?.GetCell(1).CellFormula);

destSheet.GetRow(0)?.GetCell(0).SetCellValue(10);
var evaluator = destWorkbook.GetCreationHelper()
.CreateFormulaEvaluator();

var destCell = destSheet.GetRow(0)?.GetCell(1);
evaluator.EvaluateFormulaCell(destCell);
var destCellValue = evaluator.Evaluate(destCell);

Assert.AreEqual(11, destCellValue.NumberValue);
}

[Test]
public void CopySheetToWorkbookShouldCopyMergedRegionsOver()
{
HSSFWorkbook srcWorkbook = new HSSFWorkbook();
HSSFSheet srcSheet = srcWorkbook.CreateSheet("Sheet1") as HSSFSheet;

// Set some merged regions
srcSheet.AddMergedRegion(CellRangeAddress.ValueOf("A1:B4"));
srcSheet.AddMergedRegion(CellRangeAddress.ValueOf("C1:F40"));


HSSFWorkbook destWorkbook = new HSSFWorkbook();
srcSheet.CopyTo(destWorkbook, srcSheet.SheetName, true, true);

var destSheet = destWorkbook.GetSheet("Sheet1");
Assert.NotNull(destSheet);
Assert.AreEqual(2, destSheet.MergedRegions.Count);

Assert.IsTrue(
new string[]
{
"A1:B4",
"C1:F40"
}
.SequenceEqual(
destSheet.MergedRegions
.Select(r => r.FormatAsString())));
}
}
}
92 changes: 92 additions & 0 deletions testcases/ooxml/XSSF/UserModel/TestXSSFSheetCopyTo.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,92 @@
/* ====================================================================
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.
==================================================================== */

using NPOI.SS.UserModel;
using NPOI.SS.Util;
using NPOI.XSSF.UserModel;
using NUnit.Framework;
using System;
using System.Linq;
using TestCases.SS.UserModel;

namespace TestCases.XSSF.UserModel
{
[TestFixture]
public class TestXSSFSheetCopyTo
{
[Test]
public void CopySheetToWorkbookShouldCopyFormulasOver()
{
XSSFWorkbook srcWorkbook = new XSSFWorkbook();
XSSFSheet srcSheet = srcWorkbook.CreateSheet("Sheet1") as XSSFSheet;

// Set some values
IRow row1 = srcSheet.CreateRow((short)0);
ICell cell = row1.CreateCell((short)0);
cell.SetCellValue(1);
ICell cell2 = row1.CreateCell((short)1);
cell2.SetCellFormula("A1+1");
XSSFWorkbook destWorkbook = new XSSFWorkbook();
srcSheet.CopyTo(destWorkbook, srcSheet.SheetName, true, true);

var destSheet = destWorkbook.GetSheet("Sheet1");
Assert.NotNull(destSheet);

Assert.AreEqual(1, destSheet.GetRow(0)?.GetCell(0).NumericCellValue);
Assert.AreEqual("A1+1", destSheet.GetRow(0)?.GetCell(1).CellFormula);

destSheet.GetRow(0)?.GetCell(0).SetCellValue(10);
var evaluator = destWorkbook.GetCreationHelper()
.CreateFormulaEvaluator();

var destCell = destSheet.GetRow(0)?.GetCell(1);
evaluator.EvaluateFormulaCell(destCell);
var destCellValue = evaluator.Evaluate(destCell);

Assert.AreEqual(11, destCellValue.NumberValue);
}

[Test]
public void CopySheetToWorkbookShouldCopyMergedRegionsOver()
{
XSSFWorkbook srcWorkbook = new XSSFWorkbook();
XSSFSheet srcSheet = srcWorkbook.CreateSheet("Sheet1") as XSSFSheet;

// Set some merged regions
srcSheet.AddMergedRegion(CellRangeAddress.ValueOf("A1:B4"));
srcSheet.AddMergedRegion(CellRangeAddress.ValueOf("C1:F40"));


XSSFWorkbook destWorkbook = new XSSFWorkbook();
srcSheet.CopyTo(destWorkbook, srcSheet.SheetName, true, true);

var destSheet = destWorkbook.GetSheet("Sheet1");
Assert.NotNull(destSheet);
Assert.AreEqual(2, destSheet.MergedRegions.Count);

Assert.IsTrue(
new string[]
{
"A1:B4",
"C1:F40"
}
.SequenceEqual(
destSheet.MergedRegions
.Select(r => r.FormatAsString())));
}
}
}
Loading