Skip to content

CSV import fails on OSX when EOL char is 0D #1033

Closed

Description

@justinclift

Details for the issue

When attempting to import either of the .csv files from GitHubs' recent "Open Source Survey 2017" data release, they both fail with an error message for the first line:
screen shot 2017-06-04 at 00 21 53

That error message is from negative_incidents.csv, but survey_data.csv (much larger file) exhibits the same problem. Looking through the data, there's no obvious line of double the correct length. The error message itself says there's double the number of fields, which indicates some kind of problem with the line ending not being recognised.

The data itself doesn't seem to have a problem though. The EOL character is 0D (carriage return):
screen shot 2017-06-04 at 00 34 33

Haven't yet looked at the code, but I'm guessing we're assuming Windows style newlines. If that's the case, we should probably update the import dialog to accept platform-dependent newlines. Either automatically, or ask like we do in the export dialog. Kind of thinking we should be able to auto-detect them though. 😄

Useful extra information

I'm opening this issue because:

  • DB4S has a bug

I'm using DB4S on:

  • Mac OS: (10.11.6)

I'm using DB4S version:

  • 3.9.1
  • Other: Nightly build from a few days ago (2017-05-2*)

I have also:

Activity

  1. added
    bugConfirmed bugs or reports that are very likely to be bugs.
    and removed on Jun 3, 2017
  2. changed the title [-]Display failed row number when CSV import fails[/-] [+]CSV import fails on OSX when EOL char is `0D`[/+] on Jun 3, 2017
  3. MKleusberg commented on Jun 29, 2017

    @MKleusberg
    Member

    Wow, what an issue 😆 I'm wondering what the chances of this happening are. And what a hell to debug.

    So it turns out this is not caused by the 0D. We actually support this already. You can tell by the fact that the preview in the Import CSV dialog is working correctly. Instead the issue is caused by the fact that we're not reading the CSV file entirely at once but in smaller chunks. We then loop through each chunk byte by byte and parse it. Now, if we hit a \r character we do a lookahead check if the next byte is a \n character. The problem arises when the \r character is right at the end of the current buffer, that means the following character isn't available yet. Essentially this would put the parser in a wrong state. This is by the way also the reason that smaller portions of the file (what I tried first) won't trigger the error. Anyway, the issue is now fixed by checking for this exact case and loading an extra byte into the buffer when needed 😄

    @justinclift Can you double check it's working for you, too? 😄

  4. justinclift commented on Jun 30, 2017

    @justinclift
    MemberAuthor

    Yep, will do shortly. 😄

  5. justinclift commented on Jun 30, 2017

    @justinclift
    MemberAuthor

    Just tested on Fedora 25, and the imports worked. I'll test it on the OSX Desktop in a few minutes too just to make sure, as that's what the problem originally showed up on.

  6. justinclift commented on Jun 30, 2017

    @justinclift
    MemberAuthor

    Awesome, yep this is definitely fixed now @MKleusberg. Tested it now on the OSX desktop... import went flawlessly. Thanks heaps. 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugConfirmed bugs or reports that are very likely to be bugs.

Type

No type

Projects

No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions