Bug with csv type inference

[andygrove]

Describe the bug

❯ CREATE EXTERNAL TABLE test
STORED AS CSV
WITH HEADER ROW
LOCATION '../testing/data/csv/aggregate_test_100.csv';
0 rows in set. Query took 0.102 seconds.
❯ select * from test;
ArrowError(ParseError("Error while parsing value 14857091259186476033 for column 9 at line 3"))

To Reproduce
Steps to reproduce the behavior:

Expected behavior
A clear and concise description of what you expected to happen.

Additional context
Add any other context about the problem here.

[bezbac]

Hello 👋🏻.

I haven't contributed here before but checked out this issue because of its "Good First Issue" label.
Maybe you could help me figure out what the expected behavior is.

The cell's value inside the CSV file (14857091259186476033) overflows the inferred type Int64.
So assuming the above query is supposed to work given the aggregate_test_100.csv file as input, I would suspect the column should be inferred as UInt64.
To fix this, changes would likely be needed within the arrow crate, as the CSV infer_schema functions utilizes the arrow::civ::reader::infer_reader_schema function.

Do you have any direction on how to proceed?

[alamb]

👋 @bezbac

I think looking into improving the inference in the arrow crate would be a good step

Another potential alternative to using UInt64 might be Decimal128 (though I don't want to mislead you too much here)

[kmitchener]

There's also a TODO directly related to this that could be cleaned up as part of this PR -> https://github.com/apache/arrow-datafusion/blob/master/datafusion/core/tests/sql/mod.rs#L624

[alamb]

This still happens as of datafusion 42

> CREATE EXTERNAL TABLE test
STORED AS CSV
LOCATION 'testing/data/csv/aggregate_test_100.csv';
0 row(s) fetched.
Elapsed 0.009 seconds.

> select * from test;
Arrow error: Parser error: Error while parsing value 11720144131976083864 for column 9 at line 2

[CookiePieWw]

Hi, I took a look based on @bezbac's findings, and found that arrow-csv uses regex expr to match strings and then infer ther types. The regex for Int64 accept all possible numbers instead of numbers in ranges of Int64, which caused that the inferred type actually cannot accept the strings.
(Seems github cannot attach codes through links that does not belong to this repo, so I copied here)

/// See https://github.com/apache/arrow-rs/blob/ebcc4a585136cd1d9696c38c41f71c9ced181f57/arrow-csv/src/reader/mod.rs
/// #L146-L158
    /// Order should match [`InferredDataType`]
    static ref REGEX_SET: RegexSet = RegexSet::new([
        // ...
        r"^-?(\d+)$", //INTEGER
        // ...
    ]).unwrap();

/// See https://github.com/apache/arrow-rs/blob/ebcc4a585136cd1d9696c38c41f71c9ced181f57/arrow-csv/src/reader/mod.rs
/// #L214-L223
    /// Updates the [`InferredDataType`] with the given string
    fn update(&mut self, string: &str) {
        self.packed |= if string.starts_with('"') {
            1 << 8 // Utf8
        } else if let Some(m) = REGEX_SET.matches(string).into_iter().next() {
            1 << m
        } else {
            1 << 8 // Utf8
        }
    }

One of the solutions is to change the regex expr for Int64 to match the range of it, but it seems to be a very complicate one. I told gpt to give me one but the regex expr's more than 300 chars, so I wonder if there're alternatives :)

[alamb]

I wonder if you could use a length check on the characters too (it wouldn't be perfect, but it might catch more)

As in you could bound the number of characters matched by the regexp to the number of characters in Int64::MAX (20 or so??)

[CookiePieWw]

I wonder if you could use a length check on the characters too (it wouldn't be perfect, but it might catch more)

Thanks for your suggestion! But if we want to precisely match it, we have to iter through the prefix of Int64::MAX and then we got:

922337203685477580[0-7]
9223372036854775[0-7]\d{2}
922337203685477[0-4]\d{3}
92233720368547[0-6]\d{4}

and so on...

Perhaps we can use a function for match strings here instead of direct regex here, which seems more flexible for me. If you agree on that, I'll try to submit a pr to arrow-rs for further discussions :)

[alamb]

Perhaps we can use a function for match strings here instead of direct regex here, which seems more flexible for me. If you agree on that, I'll try to submit a pr to arrow-rs for further discussions :)

That seems like a good idea to me -- with sufficient care it should also be significantly faster than a regexp based matching approach.