fix: Correctly process numeric literals with underscores

[nuno-faria]

Which issue does this PR close?

• Closes Numeric SQL literals with underscores are not correctly parsed #23877.

Rationale for this change

Be able to use numeric literals with underscores as separators, which are valid in some dialects like Postgres.

What changes are included in this PR?

The parse_sql_number method now removes underscores from numbers first before passing to the Rust parser.

Are these changes tested?

Yes.

Are there any user-facing changes?

No.

[codecov-commenter]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.86%. Comparing base (b902256) to head (e95b85b).

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #24046      +/-   ##
==========================================
- Coverage   80.86%   80.86%   -0.01%     
==========================================
  Files        1101     1101              
  Lines      375446   375474      +28     
  Branches   375446   375474      +28     
==========================================
+ Hits       303598   303614      +16     
- Misses      53758    53767       +9     
- Partials    18090    18093       +3     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Jefffrey]

i think we need to account for edge cases like multiple underscores in a row, and trailing underscores

postgres=# select 12__10;
ERROR:  trailing junk after numeric literal at or near "12__10"
LINE 1: select 12__10;
postgres=# select 12_10_;
ERROR:  trailing junk after numeric literal at or near "12_10_"
LINE 1: select 12_10_;

i think this code allows that whilst postgres forbids it

[getChan]

nit : how about retain()? for cleanup
same memory and iteration

let mut signed_number = if negative {                                                                                
    format!("-{unsigned_number}")                                                                                    
} else {                                                                                                             
    unsigned_number.to_string()                                                                                      
};                                                                                                                   
signed_number.retain(|c| c != '_'); 

[nuno-faria]

Thanks @getChan, that version looks simpler but I think it ends up doing more work. Here is a profiling that compares both with negative numbers (version_two is the original):

Here is the code I used to test:

main.rs

use std::time::Instant;

const ITERS: usize = 100_000_000;

fn version_one(input: &str, negative: bool) {
    let mut signed_number = if negative {
        format!("-{input}")
    } else {
        input.to_string()
    };

    signed_number.retain(|c| c != '_');

    let _unsigned_number = if negative {
        &signed_number[1..]
    } else {
        signed_number.as_str()
    };
}

fn version_two(input: &str, negative: bool) {
    let mut signed_number = String::with_capacity(input.len() + usize::from(negative));
    if negative {
        signed_number.push('-');
    }
    for b in input.bytes() {
        if b != b'_' {
            signed_number.push(b as char);
        }
    }

    let _unsigned_number = if negative {
        &signed_number[1..]
    } else {
        signed_number.as_str()
    };
}

fn main() {
    let input = "123_456_789_012_345_678_901_234_567_890";
    let negative = false;

    let start = Instant::now();

    for _ in 0..ITERS {
        version_one(input, negative);
    }

    let v1 = start.elapsed().as_nanos() as f64 / ITERS as f64;

    let start = Instant::now();

    for _ in 0..ITERS {
        version_two(input, negative);
    }

    let v2 = start.elapsed().as_nanos() as f64 / ITERS as f64;

    println!("version 1: {v1:.2} ns/op");
    println!("version 2: {v2:.2} ns/op");
}

Without negative numbers they are similar in performance.

[nuno-faria]

i think we need to account for edge cases like multiple underscores in a row, and trailing underscores

postgres=# select 12__10;
ERROR:  trailing junk after numeric literal at or near "12__10"
LINE 1: select 12__10;
postgres=# select 12_10_;
ERROR:  trailing junk after numeric literal at or near "12_10_"
LINE 1: select 12_10_;

i think this code allows that whilst postgres forbids it

Thanks @Jefffrey. I think that would need to be done directly in the sqlparser crate. As far as I'm aware, the numbers that reach the parse_sql_number are always valid. For example, with 10__00, the number that reaches is 10, while __10 is treated as the alias (like DuckDB does):

> select 10__00;                                                                                     
+------+
| __00 |
+------+
| 10   |
+------+

[Jefffrey]

• raised apache/datafusion-sqlparser-rs#2421 upstream