Skip to content

Commit 4373e17

Browse files
Address Copilot review comments for regional settings
- Fix FormatMoney to use string manipulation instead of float64 to preserve precision for large MONEY/SMALLMONEY values - Improve test naming in TestGetEncoding to use cp_<number> format for better readability when tests fail
1 parent 5233c2b commit 4373e17

2 files changed

Lines changed: 29 additions & 13 deletions

File tree

pkg/sqlcmd/codepage_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
package sqlcmd
55

66
import (
7+
"fmt"
78
"testing"
89

910
"github.com/stretchr/testify/assert"
@@ -211,7 +212,7 @@ func TestGetEncoding(t *testing.T) {
211212
}
212213

213214
for _, tt := range tests {
214-
t.Run(string(rune(tt.codepage)), func(t *testing.T) {
215+
t.Run(fmt.Sprintf("cp_%d", tt.codepage), func(t *testing.T) {
215216
enc, err := GetEncoding(tt.codepage)
216217
if tt.wantErr {
217218
assert.Error(t, err)

pkg/sqlcmd/regional.go

Lines changed: 27 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import (
1010

1111
"golang.org/x/text/language"
1212
"golang.org/x/text/message"
13-
"golang.org/x/text/number"
1413
)
1514

1615
// RegionalSettings provides locale-aware formatting for output when -R is used
@@ -71,30 +70,46 @@ func (r *RegionalSettings) FormatNumber(value string) string {
7170

7271
// FormatMoney formats a currency value with locale-specific formatting
7372
// Used for MONEY and SMALLMONEY types
73+
// Uses pure string manipulation to preserve precision for large values
7474
func (r *RegionalSettings) FormatMoney(value string) string {
7575
if !r.enabled || value == "" || value == "NULL" {
7676
return value
7777
}
7878

79-
// Parse the money value
79+
// MONEY/SMALLMONEY are fixed-point with 4 decimal places.
80+
// Avoid float64 to prevent rounding of large values; format via string operations.
8081
negative := strings.HasPrefix(value, "-")
8182
cleanValue := value
8283
if negative {
8384
cleanValue = value[1:]
8485
}
8586

86-
if f, err := strconv.ParseFloat(cleanValue, 64); err == nil {
87-
// Use locale-aware number formatting
88-
// Note: We use number formatting, not currency, to maintain compatibility
89-
// with ODBC sqlcmd which formats with thousand separators but not currency symbols
90-
formatted := r.printer.Sprint(number.Decimal(f, number.Scale(4)))
91-
if negative && !strings.HasPrefix(formatted, "-") {
92-
formatted = "-" + formatted
93-
}
94-
return formatted
87+
// Split into integer and fractional parts
88+
parts := strings.SplitN(cleanValue, ".", 2)
89+
intPart := parts[0]
90+
fracPart := ""
91+
if len(parts) > 1 {
92+
fracPart = parts[1]
93+
}
94+
95+
// Normalize fractional part to exactly 4 digits, matching SQL Server MONEY display
96+
if len(fracPart) == 0 {
97+
fracPart = "0000"
98+
} else if len(fracPart) < 4 {
99+
fracPart = fracPart + strings.Repeat("0", 4-len(fracPart))
100+
} else if len(fracPart) > 4 {
101+
fracPart = fracPart[:4]
95102
}
96103

97-
return value
104+
// Apply locale-specific thousand separators to the integer part
105+
formattedInt := addThousandSeparators(intPart, r.tag)
106+
107+
// Combine with locale-specific decimal separator
108+
formatted := formattedInt + getDecimalSeparator(r.tag) + fracPart
109+
if negative {
110+
formatted = "-" + formatted
111+
}
112+
return formatted
98113
}
99114

100115
// FormatDate formats a date value using locale-specific date format

0 commit comments

Comments
 (0)