Skip to content

Commit f41d2ee

Browse files
authored
fix: parquet negative DECIMAL loses sign (Issue #234) (#243)
1 parent 61306cc commit f41d2ee

2 files changed

Lines changed: 42 additions & 4 deletions

File tree

‎build.zig‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2874,6 +2874,29 @@ pub fn build(b: *std.Build) void {
28742874
const run_loader_unit_tests = b.addRunArtifact(loader_unit_tests);
28752875
unit_test_step.dependOn(&run_loader_unit_tests.step);
28762876

2877+
// Unit tests for the Parquet loader (src/parquet.zig) — decimalToText sign handling
2878+
const parquet_unit_tests = b.addTest(.{
2879+
.root_module = b.createModule(.{
2880+
.root_source_file = b.path("src/parquet.zig"),
2881+
.target = target,
2882+
.optimize = optimize,
2883+
.link_libc = true,
2884+
}),
2885+
});
2886+
parquet_unit_tests.root_module.addImport("c", translate_c.createModule());
2887+
parquet_unit_tests.root_module.addImport("zig_parquet", zig_parquet.module("parquet"));
2888+
if (bundle_sqlite) {
2889+
parquet_unit_tests.root_module.addIncludePath(b.path("lib"));
2890+
parquet_unit_tests.root_module.addCSourceFile(.{
2891+
.file = b.path("lib/sqlite3.c"),
2892+
.flags = &.{ "-DSQLITE_OMIT_LOAD_EXTENSION=1", "-DSQLITE_ENABLE_MATH_FUNCTIONS=1" },
2893+
});
2894+
} else {
2895+
parquet_unit_tests.root_module.linkSystemLibrary("sqlite3", .{});
2896+
}
2897+
const run_parquet_unit_tests = b.addRunArtifact(parquet_unit_tests);
2898+
unit_test_step.dependOn(&run_parquet_unit_tests.step);
2899+
28772900
// ─── --stats / --profile integration tests ──────────────────────────
28782901

28792902
// Integration test: --stats on basic CSV with mixed types

‎src/parquet.zig‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,13 +91,19 @@ fn decimalToText(value: i64, scale: i32, buf: *[64]u8) []const u8 {
9191
// ponytail: manual int-to-text avoids bufPrint FixedWriter issues
9292
return intToBuf(value, buf);
9393
}
94-
var pow10: i64 = 1;
94+
var pow10: u64 = 1;
9595
for (0..@as(usize, @intCast(scale))) |_| pow10 *= 10;
96-
const int_part = @divTrunc(value, pow10);
97-
const prefix = intToBuf(int_part, buf);
96+
// ponytail: sign tracked independently (Issue #234) — @divTrunc(-1, 100)
97+
// is 0, so intToBuf(int_part) alone drops the sign of |value| < pow10
98+
const neg = value < 0;
99+
const mag: u64 = @intCast(@abs(value));
100+
if (neg) buf[0] = '-';
101+
const off: usize = @intFromBool(neg);
102+
const digits = intToBuf(@as(i64, @intCast(mag / pow10)), buf[off..]);
103+
const prefix = buf[0 .. off + digits.len];
98104
buf[prefix.len] = '.';
99105
const dot_pos = prefix.len + 1;
100-
var f = @as(u64, @intCast(@abs(@rem(value, pow10))));
106+
var f = mag % pow10;
101107
var pos: usize = @intCast(scale);
102108
while (pos > 0) {
103109
pos -= 1;
@@ -137,6 +143,15 @@ fn intToBuf(value: i64, buf: []u8) []const u8 {
137143
return buf[0..end];
138144
}
139145

146+
test "decimalToText: negative sub-unit value keeps its sign (Issue #234)" {
147+
var buf: [64]u8 = undefined;
148+
try std.testing.expectEqualStrings("-0.01", decimalToText(-1, 2, &buf));
149+
try std.testing.expectEqualStrings("0.01", decimalToText(1, 2, &buf));
150+
try std.testing.expectEqualStrings("-123.45", decimalToText(-12345, 2, &buf));
151+
try std.testing.expectEqualStrings("-5", decimalToText(-5, 0, &buf));
152+
try std.testing.expectEqualStrings("0.00", decimalToText(0, 2, &buf));
153+
}
154+
140155
/// Map a Parquet physical type to a SQLite ColumnType (no logical type mapping).
141156
fn physicalToAffinity(phys: parquet.format.PhysicalType) sqlite_mod.ColumnType {
142157
return switch (phys) {

0 commit comments

Comments
 (0)