Skip to content

fwrite: fix integer-backed POSIXct writing epoch instead of ISO #3535 #7728

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.

Sign up for GitHub

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

Open
rmwood0908 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

fwrite: fix integer-backed POSIXct writing epoch instead of ISO #3535 #7728

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
16b744d
Fix fwrite POSIXct handling for INTSXP sequences
tjc234 Apr 19, 2026
4775100
fwrite: fix integer-backed POSIXct writing epoch instead of ISO #3535
rmwood0908 Apr 26, 2026
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
2 changes: 2 additions & 0 deletions NEWS.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@

9. `fread()` no longer replaces a literal header column name `"NA"` with an auto-generated `Vn` name when `na.strings` includes `"NA"`, [#5124](https://github.com/Rdatatable/data.table/issues/5124). Data rows still continue to parse `"NA"` as missing. Thanks @Mashin6 for the report and @shrektan for the fix.

10. `fwrite` with a `POSIXct` column backed by integer storage (e.g. created via `seq()`) would write epoch integers instead of ISO datetime strings, [#3535](https://github.com/Rdatatable/data.table/issues/3535). Thanks to @alistaire47 for reporting and @tjc234 and @rmwood0908 for fixing.

### Notes

1. {data.table} now depends on R 3.5.0 (2018).
Expand Down
17 changes: 17 additions & 0 deletions inst/tests/tests.Rraw
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21585,3 +21585,20 @@ close(con)
file.create(f <- tempfile())
test(2367.6, fread(file(f)), data.table(), warning="Connection has size 0.")
unlink(f)
# fwrite ignores dateTimeAs for integer-backed POSIXct created by seq() #3535
oldtz = Sys.getenv('TZ')
Sys.setenv(TZ = 'UTC')
tmp <- tempfile()
on.exit(Sys.setenv(TZ = oldtz), add = TRUE)
Comment on lines +21589 to +21592
Copy link
Copy Markdown
Member

@aitap aitap Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to adjust TZ, just give the tz = 'UTC' argument to as.POSIXct().

DT <- data.table(x = seq(as.POSIXct('1970-01-01'), by = '1 sec', length = 6))
fwrite(DT, tmp)
test(2368.1, capture.output(cat(readLines(tmp), sep = '\n')), c(
Comment on lines +21594 to +21595
Copy link
Copy Markdown
Member

@aitap aitap Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you don't give an output argument to fwrite(), it prints to the standard output. You can then test the value of capture.output(fwrite(...)) without going through a temporary file (which you won't have to remember to unlink()).

data.table:::test also has an output argument, but those are skipped when running with a non-English locale, and this test must succeed no matter whether or not message translation is enabled.

"x",
"1970-01-01T00:00:00Z",
"1970-01-01T00:00:01Z",
"1970-01-01T00:00:02Z",
"1970-01-01T00:00:03Z",
"1970-01-01T00:00:04Z",
"1970-01-01T00:00:05Z"
))
rm(DT, tmp, oldtz)
10 changes: 10 additions & 0 deletions src/fwrite.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,6 +480,16 @@ void writePOSIXct(const void *col, int64_t row, char **pch)
*pch = ch;
}

void writePOSIXctInt(const void *col, int64_t row, char **pch)
{
// integer-backed POSIXct
// before formatting, since writePOSIXct expects double storage
int32_t xi = ((const int32_t*)col)[row];
double x = (xi == INT32_MIN) ? NAN : (double)xi;
const void *tmp = &x;
writePOSIXct(tmp, 0, pch);
Comment on lines +489 to +490
Copy link
Copy Markdown
Member

@aitap aitap Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can give &x straight to writePOSIXct() without going through a temporary variable. C allows this implicit conversion because the destination type is void *.

}

// # nocov start. Covered in other.Rraw test 22, not the main suite.
void writeNanotime(const void *col, int64_t row, char **pch)
{
Expand Down
3 changes: 3 additions & 0 deletions src/fwrite.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ writer_fun_t writeITime;
writer_fun_t writeDateInt32;
writer_fun_t writeDateFloat64;
writer_fun_t writePOSIXct;
Copy link
Copy Markdown
Member

@aitap aitap Apr 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about renaming this function to writePOSIXctDouble?

writer_fun_t writePOSIXctInt;
writer_fun_t writeNanotime;
writer_fun_t writeString;
writer_fun_t writeCategString;
Expand All @@ -42,6 +43,7 @@ typedef enum { // same order as fun[] above
WF_DateInt32,
WF_DateFloat64,
WF_POSIXct,
WF_POSIXctInt,
WF_Nanotime,
WF_String,
WF_CategString,
Expand All @@ -60,6 +62,7 @@ static const int writerMaxLen[] = { // same order as fun[] and WFs above; max f
16, //&writeDateInt32
16, //&writeDateFloat64
32, //&writePOSIXct
32, //&writePOSIXctInt
48, //&writeNanotime
0, //&writeString
0, //&writeCategString
Expand Down
2 changes: 2 additions & 0 deletions src/fwriteR.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ writer_fun_t *funs[] = {
&writeDateInt32,
&writeDateFloat64,
&writePOSIXct,
&writePOSIXctInt,
&writeNanotime,
&writeString,
&writeCategString,
Expand Down Expand Up @@ -128,6 +129,7 @@ static int32_t whichWriter(SEXP column) {
if (dateTimeAs == DATETIMEAS_EPOCH) return WF_Int32;
if (INHERITS(column, char_ITime)) return WF_ITime;
if (INHERITS(column, char_Date)) return WF_DateInt32;
if (INHERITS(column, char_POSIXct)) return WF_POSIXctInt;
return WF_Int32;
case REALSXP:
if (INHERITS(column, char_nanotime)
Expand Down