fread fails w/ nrow > 1000 and out of region garbage row

[mneilly]

First, thank you for the enormous amount of effort that must go into data.table!

This issue seems to be a variant of #831. The following comment on the NEWS page stands out to me as relevant but not knowing anything about the code base maybe it is irrelevant...

fread has always jumped to the middle and to the end of the file for a much improved column type guess. The sample size is increased from 100 rows at 10 jump jump points (1,000 rows) to 100 rows at 100 jumps points (10,000 row sample).

There is a single CSV file attached that contains a header followed by 1012 rows of data and a final line of "# JUNK". This is intended to be a minimal example of the failure and not my actual use case which is multiple CSV regions within a single file as in #831.

foo.txt

For the attached CSV (named foo.txt and not foo.csv since I could not upload a csv file) the following fread() with nrow=1000 works:

> fread("/tmp/foo.csv", nrow=1000, header=TRUE, verbose=T)
Input contains no \n. Taking this to be a filename to open
File opened, filesize is 0.000008 GB.
Memory mapping ... ok
Detected eol as \n only (no \r afterwards), the UNIX and Mac standard.
Positioned on line 1 after skip or autostart
This line is the autostart and not blank so searching up for the last non-blank ... line 1
Detecting sep ... ','
Detected 2 columns. Longest stretch was from line 1 to line 30
Starting data input on line 1 (either column names or first row of data). First 10 characters: count,cach
'header' changed by user from 'auto' to TRUE
nrow set to nrows passed in (1000)
Type codes (point  0): 14
Type codes: 14 (after applying colClasses and integer64)
Type codes: 14 (after applying drop or select (if supplied)
Allocating 2 column slots (2 - 0 dropped)
Read 1000 rows. Exactly what was estimated and allocated up front
   0.000s ( 20%) Memory map (rerun may be quicker)
   0.000s ( 12%) sep and header detection
   0.000s (  2%) Count rows (wc -l)
   0.000s ( 26%) Column type detection (100 rows at 10 points)
   0.000s (  4%) Allocation of 1000x2 result (xMB) in RAM
   0.000s ( 34%) Reading data
   0.000s (  0%) Allocation for type bumps (if any), including gc time if triggered
   0.000s (  0%) Coercing data already read in type bumps (if any)
   0.000s (  2%) Changing na.strings to NA
   0.000s        Total
      count cacheid
   1:     1    blah
   2:     2    blah
   3:     3    blah
   4:     4    blah
   5:     5    blah
  ---
 996:   996    blah
 997:   997    blah
 998:   998    blah
 999:   999    blah
1000:  1000    blah
>

but the following fread() with nrow=1001 does not:

> fread("/tmp/foo.csv", nrow=1001, header=TRUE, verbose=T)
Input contains no \n. Taking this to be a filename to open
File opened, filesize is 0.000008 GB.
Memory mapping ... ok
Detected eol as \n only (no \r afterwards), the UNIX and Mac standard.
Positioned on line 1 after skip or autostart
This line is the autostart and not blank so searching up for the last non-blank ... line 1
Detecting sep ... ','
Detected 2 columns. Longest stretch was from line 1 to line 30
Starting data input on line 1 (either column names or first row of data). First 10 characters: count,cach
'header' changed by user from 'auto' to TRUE
nrow set to nrows passed in (1001)
Type codes (point  0): 14
Type codes (point  1): 14
Type codes (point  2): 14
Type codes (point  3): 14
Type codes (point  4): 14
Type codes (point  5): 14
Type codes (point  6): 14
Type codes (point  7): 14
Type codes (point  8): 14
Error in fread("/tmp/foo.csv", nrow = 1001, header = TRUE, verbose = T) :
  Expected sep (',') but new line, EOF (or other non printing character) ends field 0 when detecting types from point 9: # JUNK
>  

Adding a "," after "# Junk" causes the fread to succeed.

It appears that as long as the row with '# Junk' has at least as many columns as the specified region of interest, fread works, but if it has fewer columns it does not work. I have verified that using read.csv() with the same CSV and parameters works.

The following is the output from sessionInfo():

> sessionInfo()
R version 3.3.3 (2017-03-06)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Debian GNU/Linux 9 (stretch)

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C
 [3] LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8
 [5] LC_MONETARY=en_US.UTF-8    LC_MESSAGES=en_US.UTF-8
 [7] LC_PAPER=en_US.UTF-8       LC_NAME=C
 [9] LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] data.table_1.10.4-2

loaded via a namespace (and not attached):
 [1] colorspace_1.3-2 scales_0.4.1     lazyeval_0.2.0   plyr_1.8.4
 [5] tools_3.3.3      gtable_0.2.0     tibble_1.3.3     Rcpp_0.12.14
 [9] ggplot2_2.2.1    grid_3.3.3       rlang_0.1.1      munsell_0.4.3

[mneilly]

I thought I would test master to see if the problem goes away. The specific CSV file previously attached works but in general the use of skip and nrow seems to have problems. Sometimes, the wrong data is returned.

FWIW - I've attached a bash script (as .txt file) which runs some simple random testing and reports results. The script never fails with read.csv() but does with fread().

test-data-table.sh

I've also attached another example CSV (as .txt) file which includes a case that returns the wrong data.

test.csv

In the output below, the third fread() returns the wrong data:

> fread("/tmp/test.csv", skip=0, nrow=8)
Error in fread("/tmp/test.csv", skip = 0, nrow = 8) :
  Line 21 has more than the expected 3 fields. Stopped on <<,col4,col5>> at character 15. Consider setting 'comment.char=' if there is a trailing comment to be ignored. First 500 characters of line: <<col1,col2,col3,col4,col5>>
> fread("/tmp/test.csv", skip=9, nrow=10)                                                                                                             
Error in fread("/tmp/test.csv", skip = 9, nrow = 10) :
  Line 21 has more than the expected 3 fields. Stopped on <<,col4,col5>> at character 15. Consider setting 'comment.char=' if there is a trailing comment to be ignored. First 500 characters of line: <<col1,col2,col3,col4,col5>>
> fread("/tmp/test.csv", skip=20, nrow=3)                                                                                                             
      col1       col2        col3           col4       col5     col6         col7      col8   col9       col10
1:   Kmart Arkansan's Gilchrist's         Johann   Skippy's   braces      Frankel Patrica's bulled Spielberg's
2: chiming   Theodora accelerated    bricklayers Wilhelmina    books     blotches     chink   I'll       Judas
3:   buffs     boughs     cookout childbearing's    blights budgie's cornflower's   caboose Cedric       azure
>

The number of columns for the region starting at line 21 is 5 not 10:

$ grep -n ^col /tmp/test.csv
1:col1,col2
10:col1,col2,col3
21:col1,col2,col3,col4,col5
25:col1,col2,col3,col4,col5,col6,col7,col8,col9,col10
33:col1,col2,col3,col4,col5,col6,col7

The following is the output from sessionInfo():

> sessionInfo()
R version 3.3.3 (2017-03-06)
Platform: x86_64-pc-linux-gnu (64-bit)
Running under: Debian GNU/Linux 9 (stretch)

locale:
 [1] LC_CTYPE=en_US.UTF-8       LC_NUMERIC=C               LC_TIME=en_US.UTF-8        LC_COLLATE=en_US.UTF-8     LC_MONETARY=en_US.UTF-8
 [6] LC_MESSAGES=en_US.UTF-8    LC_PAPER=en_US.UTF-8       LC_NAME=C                  LC_ADDRESS=C               LC_TELEPHONE=C
[11] LC_MEASUREMENT=en_US.UTF-8 LC_IDENTIFICATION=C

attached base packages:
[1] stats     graphics  grDevices utils     datasets  methods   base

other attached packages:
[1] devtools_1.13.1   data.table_1.10.5

loaded via a namespace (and not attached):
 [1] colorspace_1.3-2 scales_0.4.1     lazyeval_0.2.0   plyr_1.8.4       withr_1.0.2      gtable_0.2.0     tibble_1.3.3     memoise_1.1.0
 [9] Rcpp_0.12.14     ggplot2_2.2.1    grid_3.3.3       digest_0.6.12    rlang_0.1.1      munsell_0.4.3
>

[mneilly]

FWIW - I made a local change that mmaps only the requested region of the file and updated the test script to compare the fread() result against read.csv(). With the changes the script doesn't catch any failures.

test-data-table.sh

diff --git a/src/fread.c b/src/fread.c
index 2dfad494..84b735df 100644
--- a/src/fread.c
+++ b/src/fread.c
@@ -69,6 +69,7 @@ static double NA_FLOAT64;  // takes fread.h:NA_FLOAT64_VALUE

 // Private globals so they can be cleaned up both on error and on successful return
 static void *mmp = NULL;
+static size_t mmpSize = 0; // size mapped by mmap (might not be the same as fileSize)
 static size_t fileSize;
 static int8_t *type = NULL, *tmpType = NULL, *size = NULL;
 static lenOff *colNames = NULL;
@@ -143,7 +144,7 @@ bool freadCleanup(void)
       if (!UnmapViewOfFile(mmp))
         DTPRINT("System error %d unmapping view of file\n", GetLastError());
     #else
-      if (munmap(mmp, fileSize))
+      if (munmap(mmp, mmpSize))
         DTPRINT("System errno %d unmapping file: %s\n", errno, strerror(errno));
     #endif
     mmp = NULL;
@@ -1132,6 +1133,62 @@ int freadMain(freadMainArgs _args) {
       // TO DO?: MAP_HUGETLB for Linux but seems to need admin to setup first. My Hugepagesize is 2MB (>>2KB, so promising)
       //         https://www.kernel.org/doc/Documentation/vm/hugetlbpage.txt
       mmp = mmap(NULL, fileSize, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, 0);  // COW for last page finalByte
+
+      // If skip=0 and nrow=0 these won't change
+      off_t sofOffset = 0; // adjusts sof based on page alignment of mmap'd file
+      mmpSize = fileSize;  // tracks mmap'd size for use in munmap()
+
+      {
+          // If the user requested a particular region of the file
+          // unmap the file and map only the requested region
+
+          off_t mapOffset = 0;
+          size_t mapLen = 0;
+          int nline = 0;
+
+          const char* ch = (const char *)mmp;
+
+          eof = (const char *)mmp + fileSize;
+
+          // Determine the offset into the file after skipping skip lines
+
+          if (args.skipNrow>0) {
+              while (ch<eof && nline<args.skipNrow) { mapOffset++; nline+=(*ch++=='\n'); }
+              if (ch>eof) STOP("skip=%llu but the input only has %llu line%s", (llu)args.skipNrow, (llu)nline, nline>1?"s":"");
+          }
+
+          // Determine the size of the mapping based on nrow
+
+          nline = 0;
+          if (args.nrowLimit>0) {
+              if (args.header) nline--;
+              while (ch<eof && nline<args.nrowLimit) { mapLen++; nline+=(*ch++=='\n'); }
+              if (ch>eof) STOP("nrow=%llu but the input only has %llu line%s", (llu)args.nrowLimit, (llu)nline, nline>1?"s":"");
+          }
+
+          // If either the mapping size of offset have changed, unmap and map only the specified region
+
+          if (mapLen != fileSize || mapOffset != 0) {
+              // mmap requires offset to be page aligned
+              size_t pgSize = sysconf(_SC_PAGESIZE);
+              off_t adjOffset = (mapOffset/pgSize) * pgSize;
+              int adjLen = mapLen + (mapOffset - adjOffset);
+
+              // Set fileSize to the new mapping size
+              if (munmap(mmp, fileSize))
+                  DTPRINT("System errno %d unmapping file: %s\n", errno, strerror(errno));
+              fileSize = mapLen;
+
+              // Map the new region and adjust the starting pointer by sub-page offset
+              mmp = mmap(NULL, adjLen, PROT_READ|PROT_WRITE, MAP_PRIVATE, fd, adjOffset);
+              mmpSize = adjLen;
+              sofOffset = (mapOffset % pgSize);
+
+              if (mmp == MAP_FAILED)
+                  STOP("Could not remap input file based on requested region\n");
+          }
+      }
+
       close(fd);  // we don't need to keep file handle open
       if (mmp == MAP_FAILED) {
     #else
@@ -1165,7 +1222,7 @@ int freadMain(freadMainArgs _args) {
       STOP("Opened %s file ok but could not memory map it. This is a %dbit process. %s.", filesize_to_str(fileSize), nbit,
            nbit<=32 ? "Please upgrade to 64bit" : "There is probably not enough contiguous virtual memory available");
     }
-    sof = (const char*) mmp;
+    sof = (const char*) mmp + sofOffset;
     if (verbose) DTPRINT("  Memory mapped ok\n");
   } else {
     STOP("Neither `input` nor `filename` are given, nothing to read.");
@@ -1300,11 +1357,13 @@ int freadMain(freadMainArgs _args) {
     ch = pos;
   }
   // Skip the first `skipNrow` lines of input.
+  #if 0
   else if (args.skipNrow>0) {
     while (ch<eof && row1line<=args.skipNrow) row1line+=(*ch++=='\n');
     if (ch>=eof) STOP("skip=%llu but the input only has %llu line%s", (llu)args.skipNrow, (llu)row1line, row1line>1?"s":"");
     pos = ch;
   }
+#endif

   // skip blank input at the start
   const char *lineStart = ch;
@@ -2125,7 +2184,7 @@ int freadMain(freadMainArgs _args) {
         // The goal here is to handle the rare case of files that don't end with newline AND are also exactly a multiple of page size, PR#2200.
         // All because mmap fileSize+1 doesn't work on Mac and Windows when fileSize%pageSize==0 (bus error on read and write)
         if (finalByte && tch==eof) {
-          if (j<=0 || *eof!='\0' || !args.filename || fieldStart<=(char *)mmp) {
+          if (j<=0 || *eof!='\0' || !args.filename || fieldStart<=(char *)sof) {
             #pragma omp critical
             if (!stopTeam) {
               stopTeam = true;

[mattdowle]

Thanks for all the detailed info and attempts here! As luck would have it, I've just merged a major PR into fread 2 hours ago (#2623) that fixes exactly this.
I've just tried latest master on your file (thanks for providing) and it all works now. Please try again and I'm sure it's now fixed. If not, please reopen this issue.

> fread("~/Downloads/foo.txt")
      count text
   1:     1 blah
   2:     2 blah
   3:     3 blah
   4:     4 blah
   5:     5 blah
  ---           
 997:   997 blah
 998:   998 blah
 999:   999 blah
1000:  1000 blah
1001:  1001 blah
Warning message:
In fread("~/Downloads/foo.txt") : Discarded single-line footer: <<# JUNK>>

> fread("~/Downloads/foo.txt", nrow=1000)
      count text
   1:     1 blah
   2:     2 blah
   3:     3 blah
   4:     4 blah
   5:     5 blah
  ---           
 996:   996 blah
 997:   997 blah
 998:   998 blah
 999:   999 blah
1000:  1000 blah

> fread("~/Downloads/foo.txt", nrow=1001)
      count text
   1:     1 blah
   2:     2 blah
   3:     3 blah
   4:     4 blah
   5:     5 blah
  ---           
 997:   997 blah
 998:   998 blah
 999:   999 blah
1000:  1000 blah
1001:  1001 blah

> fread("~/Downloads/foo.txt", nrow=1002)
      count text
   1:     1 blah
   2:     2 blah
   3:     3 blah
   4:     4 blah
   5:     5 blah
  ---           
 997:   997 blah
 998:   998 blah
 999:   999 blah
1000:  1000 blah
1001:  1001 blah
Warning message:
In fread("~/Downloads/foo.txt", nrow = 1002) :
  Discarded single-line footer: <<# JUNK>>

[mneilly]

Great! Thx! Looks pretty solid to me. My script continues to run without errors. 👍