Found on v0.53.0 (bd93ba8) by writing one number into eleven cells of a sheet, giving each cell a different number format, and loading the sheet.
Phenomenon
The number a cell of an XLSX sheet loads as depends on the format the sheet would display it in, so one stored value becomes seven different values and three different column types. Every cell below holds the double 1234.5, apart from the two percent cells, which hold 0.5:
general declared=REAL value=1234.5
integer declared=INTEGER value=1235
two_decimals declared=REAL value=1234.5
thousands declared=TEXT value="1,235"
thousands_dec declared=TEXT value="1,234.50"
currency declared=REAL value=1234.5
percent declared=TEXT value="50%"
percent_dec declared=TEXT value="50.00%"
scientific declared=REAL value=1230
fraction declared=TEXT value="1234 1/2"
accounting declared=TEXT value=" $1,234.50 "
Two of these are wrong as numbers rather than merely as spellings: a cell displayed in scientific notation loads as 1230, which is 1234.5 read back from the three digits the display shows, and a cell displayed as a whole number loads as 1235. A query summing either column answers a number that is in no file. The rest lose the column instead: a sheet of percentages or of thousands-separated amounts is TEXT, so SUM and AVG over it answer 0 and ORDER BY sorts it lexically -- the same fault the date handling exists to prevent, since a workbook stores a date as a number too and this package rewrites those into ISO 8601 for exactly this reason.
Reproduction
package repro
import (
"context"
"path/filepath"
"testing"
"github.com/nao1215/filesql"
"github.com/xuri/excelize/v2"
)
func TestNumberFormatDecidesTheValue(t *testing.T) {
f := excelize.NewFile()
defer f.Close()
sheet := f.GetSheetName(0)
for i, numFmt := range []int{0, 1, 3, 9, 11} {
head, _ := excelize.CoordinatesToCellName(i+1, 1)
if err := f.SetCellStr(sheet, head, "c"); err != nil {
t.Fatal(err)
}
cell, _ := excelize.CoordinatesToCellName(i+1, 2)
value := 1234.5
if numFmt == 9 {
value = 0.5
}
if err := f.SetCellValue(sheet, cell, value); err != nil {
t.Fatal(err)
}
style, err := f.NewStyle(&excelize.Style{NumFmt: numFmt})
if err != nil {
t.Fatal(err)
}
if err := f.SetCellStyle(sheet, cell, cell, style); err != nil {
t.Fatal(err)
}
}
book := filepath.Join(t.TempDir(), "fmt.xlsx")
if err := f.SaveAs(book); err != nil {
t.Fatal(err)
}
db, err := filesql.OpenContext(context.Background(), book)
if err != nil {
t.Fatal(err)
}
defer db.Close()
// Read the one row and log each column with its type.
}
The full probe prints the table above; the two lines that matter are "scientific declared=REAL value=1230" for a cell holding 1234.5 and "integer declared=INTEGER value=1235" for the same number.
Expected behavior
A numeric cell should load the number the file stores, whatever format the sheet would draw it in, so all eleven cells above load 1234.5 or 0.5 in a REAL column. A format is presentation: it says how a spreadsheet paints a number, not what the number is, and this package's job is the data. That is already the reasoning the date handling is written from -- internal/reader/xlsx_date.go says a workbook "stores a date as a serial number and a number format" and rewrites those cells so ORDER BY does not sort them lexically -- and a percentage or a thousands-separated amount is the same situation with the same consequence. The narrow half of this is not a judgment call at all: a cell whose display is a plain number (general, a decimal count, a whole number, scientific) must load the stored number, since the display is the same number and the difference is only how many digits it shows. What to do with a display that is not a plain number -- a percentage, a currency, an accounting amount, a fraction -- is a decision to record in the documentation either way: load the stored number and let a caller format it in SQL, or keep loading the displayed text and say so under Column Types.
Cause (guess)
internal/reader/xlsx.go ReadSheet reads the sheet with excelize's GetRows, which renders every cell the way a spreadsheet would display it, and the loader then infers a column type from that rendering. The stored value is one call away: internal/reader/xlsx_date.go already reads cells with excelize.Options{RawCellValue: true}. The date pass in that file is the model for the fix -- it walks the cells the style table says could be dates and rewrites them -- and the same walk can carry the numbers, with the cell's type (excelize.CellTypeUnset or CellTypeNumber, which storesASerial already asks) telling a stored number from a stored string so a text cell spelling "1,235" is left alone.
Tests to add
- In the XLSX reader's test file, a table of one value under each built-in number format (0, 1, 2, 3, 4, 8, 9, 10, 11, 12, 44) asserting the loaded value equals the stored number, and the column type is the one that number's spelling calls for.
- Cases that pin what must not change: a date-formatted cell still loads as ISO 8601, a text cell holding "1,235" still loads as that text, a boolean cell still loads as TRUE or FALSE, and a cell holding an error value still loads as "#DIV/0!".
- A case per stored type rather than per format, since the fix keys on the cell's type: a shared string, an inline string, a boolean and a number under a date format must each come back the way they do now.
- An aggregate case, which is the symptom a caller meets: a column of percent-formatted cells summed in SQL answers the sum of the stored numbers rather than 0.
Found on v0.53.0 (bd93ba8) by writing one number into eleven cells of a sheet, giving each cell a different number format, and loading the sheet.
Phenomenon
The number a cell of an XLSX sheet loads as depends on the format the sheet would display it in, so one stored value becomes seven different values and three different column types. Every cell below holds the double 1234.5, apart from the two percent cells, which hold 0.5:
Two of these are wrong as numbers rather than merely as spellings: a cell displayed in scientific notation loads as 1230, which is 1234.5 read back from the three digits the display shows, and a cell displayed as a whole number loads as 1235. A query summing either column answers a number that is in no file. The rest lose the column instead: a sheet of percentages or of thousands-separated amounts is TEXT, so SUM and AVG over it answer 0 and ORDER BY sorts it lexically -- the same fault the date handling exists to prevent, since a workbook stores a date as a number too and this package rewrites those into ISO 8601 for exactly this reason.
Reproduction
The full probe prints the table above; the two lines that matter are "scientific declared=REAL value=1230" for a cell holding 1234.5 and "integer declared=INTEGER value=1235" for the same number.
Expected behavior
A numeric cell should load the number the file stores, whatever format the sheet would draw it in, so all eleven cells above load 1234.5 or 0.5 in a REAL column. A format is presentation: it says how a spreadsheet paints a number, not what the number is, and this package's job is the data. That is already the reasoning the date handling is written from -- internal/reader/xlsx_date.go says a workbook "stores a date as a serial number and a number format" and rewrites those cells so ORDER BY does not sort them lexically -- and a percentage or a thousands-separated amount is the same situation with the same consequence. The narrow half of this is not a judgment call at all: a cell whose display is a plain number (general, a decimal count, a whole number, scientific) must load the stored number, since the display is the same number and the difference is only how many digits it shows. What to do with a display that is not a plain number -- a percentage, a currency, an accounting amount, a fraction -- is a decision to record in the documentation either way: load the stored number and let a caller format it in SQL, or keep loading the displayed text and say so under Column Types.
Cause (guess)
internal/reader/xlsx.go ReadSheet reads the sheet with excelize's GetRows, which renders every cell the way a spreadsheet would display it, and the loader then infers a column type from that rendering. The stored value is one call away: internal/reader/xlsx_date.go already reads cells with excelize.Options{RawCellValue: true}. The date pass in that file is the model for the fix -- it walks the cells the style table says could be dates and rewrites them -- and the same walk can carry the numbers, with the cell's type (excelize.CellTypeUnset or CellTypeNumber, which storesASerial already asks) telling a stored number from a stored string so a text cell spelling "1,235" is left alone.
Tests to add