From d802e92d4678350f0fab1a80e62a8c4e0e8112e6 Mon Sep 17 00:00:00 2001 From: dengyi Date: Thu, 7 Aug 2025 11:33:59 +0800 Subject: [PATCH 1/3] feat: add IterateColumns method for improved performance --- rows.go | 89 ++++++++++++++++++++++++++++++++++++++++++++++++++++ rows_test.go | 49 +++++++++++++++++++++++++++++ 2 files changed, 138 insertions(+) diff --git a/rows.go b/rows.go index 790718cba1..949768faca 100644 --- a/rows.go +++ b/rows.go @@ -197,6 +197,95 @@ func (rows *Rows) Columns(opts ...Options) ([]string, error) { return rowIterator.cells, rowIterator.err } +// ColumnIteratorFunc defines a function type for iterating over columns in a row. +// The function receives the column index (0-based) and the cell value. +// Return a non-nil error to stop the iteration. +type ColumnIteratorFunc func(index int, cell string) error + +// The iterator function receives the column index (0-based) and the cell value. +// If the iterator function returns a non-nil error, the iteration stops and +// that error is returned. +// +// Example usage: +// +// err := rows.IterateColumns(func(index int, cell string) error { +// fmt.Printf("Column %d: %s\n", index, cell) +// return nil +// }) +func (rows *Rows) IterateColumns(f ColumnIteratorFunc, opts ...Options) error { + if rows.curRow > rows.seekRow { + return nil + } + var rowIterator rowXMLIterator + var token xml.Token + rows.rawCellValue = rows.f.getOptions(opts...).RawCellValue + if rows.sst, rowIterator.err = rows.f.sharedStringsReader(); rowIterator.err != nil { + return rowIterator.err + } + columnIndex := 0 + for { + if rows.token != nil { + token = rows.token + } else if token, _ = rows.decoder.Token(); token == nil { + break + } + switch xmlElement := token.(type) { + case xml.StartElement: + rowIterator.inElement = xmlElement.Name.Local + if rowIterator.inElement == "row" { + rowNum := 0 + if rowNum, rowIterator.err = attrValToInt("r", xmlElement.Attr); rowNum != 0 { + rows.curRow = rowNum + } else if rows.token == nil { + rows.curRow++ + } + rows.token = token + rows.seekRowOpts = extractRowOpts(xmlElement.Attr) + if rows.curRow > rows.seekRow { + rows.token = nil + return nil + } + } + if rowIterator.inElement == "c" { + rowIterator.cellCol++ + colCell := xlsxC{} + colCell.cellXMLHandler(rows.decoder, &xmlElement) + if colCell.R != "" { + if rowIterator.cellCol, _, rowIterator.err = CellNameToCoordinates(colCell.R); rowIterator.err != nil { + rows.token = nil + return rowIterator.err + } + } + for columnIndex < rowIterator.cellCol-1 { + if err := f(columnIndex, ""); err != nil { + rows.token = nil + return err + } + columnIndex++ + } + if val, _ := colCell.getValueFrom(rows.f, rows.sst, rows.rawCellValue); val != "" || colCell.F != nil { + if err := f(columnIndex, val); err != nil { + rows.token = nil + return err + } + } else { + if err := f(columnIndex, ""); err != nil { + rows.token = nil + return err + } + } + columnIndex++ + } + rows.token = nil + case xml.EndElement: + if xmlElement.Name.Local == "sheetData" { + return nil + } + } + } + return nil +} + // extractRowOpts extract row element attributes. func extractRowOpts(attrs []xml.Attr) RowOpts { rowOpts := RowOpts{Height: defaultRowHeight} diff --git a/rows_test.go b/rows_test.go index 8dba425ecc..085bfbfe52 100644 --- a/rows_test.go +++ b/rows_test.go @@ -149,6 +149,55 @@ func TestRowsError(t *testing.T) { assert.NoError(t, f.Close()) } +func TestIterateColumns(t *testing.T) { + f := NewFile() + assert.NoError(t, f.SetCellValue("Sheet1", "A1", "A1")) + assert.NoError(t, f.SetCellValue("Sheet1", "C1", "C1")) + assert.NoError(t, f.SetCellValue("Sheet1", "D1", "D1")) + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + assert.True(t, rows.Next()) + var iteratedCells []string + err = rows.IterateColumns(func(index int, cell string) error { + iteratedCells = append(iteratedCells, cell) + return nil + }) + assert.NoError(t, err) + rows2, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows2.Close() + assert.True(t, rows2.Next()) + columns, err := rows2.Columns() + assert.NoError(t, err) + assert.Equal(t, columns, iteratedCells) + assert.Equal(t, []string{"A1", "", "C1", "D1"}, iteratedCells) +} + +func TestIterateColumnsEarlyTermination(t *testing.T) { + f := NewFile() + assert.NoError(t, f.SetCellValue("Sheet1", "A1", "A1")) + assert.NoError(t, f.SetCellValue("Sheet1", "B1", "B1")) + assert.NoError(t, f.SetCellValue("Sheet1", "C1", "C1")) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + assert.True(t, rows.Next()) + var collectedCells []string + err = rows.IterateColumns(func(index int, cell string) error { + collectedCells = append(collectedCells, cell) + if index == 1 { + return fmt.Errorf("stop") // stop at second column + } + return nil + }) + assert.EqualError(t, err, "stop") + assert.Equal(t, []string{"A1", "B1"}, collectedCells) +} + func TestRowHeight(t *testing.T) { f := NewFile() sheet1 := f.GetSheetName(0) From 60235017e4e1402675bbeeafad67670f1999abe5 Mon Sep 17 00:00:00 2001 From: dengyi Date: Thu, 7 Aug 2025 14:21:10 +0800 Subject: [PATCH 2/3] fix:add more tests --- rows_test.go | 149 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 149 insertions(+) diff --git a/rows_test.go b/rows_test.go index 085bfbfe52..91570e643b 100644 --- a/rows_test.go +++ b/rows_test.go @@ -198,6 +198,155 @@ func TestIterateColumnsEarlyTermination(t *testing.T) { assert.Equal(t, []string{"A1", "B1"}, collectedCells) } +func TestIterateColumnsWithFormula(t *testing.T) { + f := NewFile() + + // Test with formula cell + assert.NoError(t, f.SetCellFormula("Sheet1", "A1", "=1+2")) + assert.NoError(t, f.SetCellValue("Sheet1", "B1", "text")) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + assert.True(t, rows.Next()) + + // Test normal mode + var normalCells []string + err = rows.IterateColumns(func(index int, cell string) error { + normalCells = append(normalCells, cell) + return nil + }) + assert.NoError(t, err) + assert.Equal(t, 2, len(normalCells)) + assert.Equal(t, "text", normalCells[1]) + + // Test with Options parameter coverage + rows2, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows2.Close() + assert.True(t, rows2.Next()) + + var rawCells []string + err = rows2.IterateColumns(func(index int, cell string) error { + rawCells = append(rawCells, cell) + return nil + }, Options{RawCellValue: true}) + assert.NoError(t, err) + assert.True(t, len(rawCells) >= 1) // Just ensure we get some cells +} + +func TestIterateColumnsErrorCases(t *testing.T) { + f := NewFile() + + // Test with invalid shared strings + f.SharedStrings = nil + f.Pkg.Store(defaultXMLPathSharedStrings, MacintoshCyrillicCharset) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + // Test sharedStringsReader error path + err = rows.IterateColumns(func(index int, cell string) error { + return nil + }) + assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8") +} + +func TestIterateColumnsSkipRows(t *testing.T) { + f := NewFile() + assert.NoError(t, f.SetCellValue("Sheet1", "A1", "row1")) + assert.NoError(t, f.SetCellValue("Sheet1", "A2", "row2")) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + // Skip first row + assert.True(t, rows.Next()) + + // Move to second row + assert.True(t, rows.Next()) + + var cells []string + err = rows.IterateColumns(func(index int, cell string) error { + cells = append(cells, cell) + return nil + }) + assert.NoError(t, err) + assert.Equal(t, []string{"row2"}, cells) +} + +func TestIterateColumnsEmptyCallback(t *testing.T) { + f := NewFile() + assert.NoError(t, f.SetCellValue("Sheet1", "A1", "")) // Empty cell + assert.NoError(t, f.SetCellValue("Sheet1", "B1", "B1")) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + assert.True(t, rows.Next()) + + var cells []string + var indices []int + err = rows.IterateColumns(func(index int, cell string) error { + cells = append(cells, cell) + indices = append(indices, index) + // Test early termination in empty cell callback + if index == 0 && cell == "" { + return fmt.Errorf("empty cell error") + } + return nil + }) + assert.EqualError(t, err, "empty cell error") + assert.Equal(t, []string{""}, cells) + assert.Equal(t, []int{0}, indices) +} + +func TestIterateColumnsBoundaryConditions(t *testing.T) { + f := NewFile() + assert.NoError(t, f.SetCellValue("Sheet1", "A1", "A1")) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + // Test curRow > seekRow condition + rows.curRow = 10 + rows.seekRow = 5 + + err = rows.IterateColumns(func(index int, cell string) error { + t.Error("Should not be called") + return nil + }) + assert.NoError(t, err) // Should return nil when curRow > seekRow +} + +func TestIterateColumnsXMLStructure(t *testing.T) { + f := NewFile() + + // Create test data to ensure XML structure coverage + assert.NoError(t, f.SetCellValue("Sheet1", "A1", "test")) + assert.NoError(t, f.SetCellFormula("Sheet1", "B1", "=SUM(1,2)")) + + rows, err := f.Rows("Sheet1") + assert.NoError(t, err) + defer rows.Close() + + assert.True(t, rows.Next()) + + // This should exercise various XML parsing paths + var cellCount int + err = rows.IterateColumns(func(index int, cell string) error { + cellCount++ + return nil + }) + assert.NoError(t, err) + assert.True(t, cellCount >= 2) +} + func TestRowHeight(t *testing.T) { f := NewFile() sheet1 := f.GetSheetName(0) From afebad2b015b861fc7528a0117ff87d829c3ba82 Mon Sep 17 00:00:00 2001 From: dengyi Date: Thu, 7 Aug 2025 14:23:56 +0800 Subject: [PATCH 3/3] fix --- rows_test.go | 151 +-------------------------------------------------- 1 file changed, 1 insertion(+), 150 deletions(-) diff --git a/rows_test.go b/rows_test.go index 91570e643b..628fa1e487 100644 --- a/rows_test.go +++ b/rows_test.go @@ -190,7 +190,7 @@ func TestIterateColumnsEarlyTermination(t *testing.T) { err = rows.IterateColumns(func(index int, cell string) error { collectedCells = append(collectedCells, cell) if index == 1 { - return fmt.Errorf("stop") // stop at second column + return fmt.Errorf("stop") } return nil }) @@ -198,155 +198,6 @@ func TestIterateColumnsEarlyTermination(t *testing.T) { assert.Equal(t, []string{"A1", "B1"}, collectedCells) } -func TestIterateColumnsWithFormula(t *testing.T) { - f := NewFile() - - // Test with formula cell - assert.NoError(t, f.SetCellFormula("Sheet1", "A1", "=1+2")) - assert.NoError(t, f.SetCellValue("Sheet1", "B1", "text")) - - rows, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows.Close() - - assert.True(t, rows.Next()) - - // Test normal mode - var normalCells []string - err = rows.IterateColumns(func(index int, cell string) error { - normalCells = append(normalCells, cell) - return nil - }) - assert.NoError(t, err) - assert.Equal(t, 2, len(normalCells)) - assert.Equal(t, "text", normalCells[1]) - - // Test with Options parameter coverage - rows2, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows2.Close() - assert.True(t, rows2.Next()) - - var rawCells []string - err = rows2.IterateColumns(func(index int, cell string) error { - rawCells = append(rawCells, cell) - return nil - }, Options{RawCellValue: true}) - assert.NoError(t, err) - assert.True(t, len(rawCells) >= 1) // Just ensure we get some cells -} - -func TestIterateColumnsErrorCases(t *testing.T) { - f := NewFile() - - // Test with invalid shared strings - f.SharedStrings = nil - f.Pkg.Store(defaultXMLPathSharedStrings, MacintoshCyrillicCharset) - - rows, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows.Close() - - // Test sharedStringsReader error path - err = rows.IterateColumns(func(index int, cell string) error { - return nil - }) - assert.EqualError(t, err, "XML syntax error on line 1: invalid UTF-8") -} - -func TestIterateColumnsSkipRows(t *testing.T) { - f := NewFile() - assert.NoError(t, f.SetCellValue("Sheet1", "A1", "row1")) - assert.NoError(t, f.SetCellValue("Sheet1", "A2", "row2")) - - rows, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows.Close() - - // Skip first row - assert.True(t, rows.Next()) - - // Move to second row - assert.True(t, rows.Next()) - - var cells []string - err = rows.IterateColumns(func(index int, cell string) error { - cells = append(cells, cell) - return nil - }) - assert.NoError(t, err) - assert.Equal(t, []string{"row2"}, cells) -} - -func TestIterateColumnsEmptyCallback(t *testing.T) { - f := NewFile() - assert.NoError(t, f.SetCellValue("Sheet1", "A1", "")) // Empty cell - assert.NoError(t, f.SetCellValue("Sheet1", "B1", "B1")) - - rows, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows.Close() - - assert.True(t, rows.Next()) - - var cells []string - var indices []int - err = rows.IterateColumns(func(index int, cell string) error { - cells = append(cells, cell) - indices = append(indices, index) - // Test early termination in empty cell callback - if index == 0 && cell == "" { - return fmt.Errorf("empty cell error") - } - return nil - }) - assert.EqualError(t, err, "empty cell error") - assert.Equal(t, []string{""}, cells) - assert.Equal(t, []int{0}, indices) -} - -func TestIterateColumnsBoundaryConditions(t *testing.T) { - f := NewFile() - assert.NoError(t, f.SetCellValue("Sheet1", "A1", "A1")) - - rows, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows.Close() - - // Test curRow > seekRow condition - rows.curRow = 10 - rows.seekRow = 5 - - err = rows.IterateColumns(func(index int, cell string) error { - t.Error("Should not be called") - return nil - }) - assert.NoError(t, err) // Should return nil when curRow > seekRow -} - -func TestIterateColumnsXMLStructure(t *testing.T) { - f := NewFile() - - // Create test data to ensure XML structure coverage - assert.NoError(t, f.SetCellValue("Sheet1", "A1", "test")) - assert.NoError(t, f.SetCellFormula("Sheet1", "B1", "=SUM(1,2)")) - - rows, err := f.Rows("Sheet1") - assert.NoError(t, err) - defer rows.Close() - - assert.True(t, rows.Next()) - - // This should exercise various XML parsing paths - var cellCount int - err = rows.IterateColumns(func(index int, cell string) error { - cellCount++ - return nil - }) - assert.NoError(t, err) - assert.True(t, cellCount >= 2) -} - func TestRowHeight(t *testing.T) { f := NewFile() sheet1 := f.GetSheetName(0)