Add APIs to reuse token buffers in Tokenizer

[0rphon]

This is a super simple PR that adds two new methods to aid in the reuse of token buffers.

• Tokenizer::tokenize_with_location_into: operates identically to Tokenizer::tokenize_with_location except it allows you to supply your own buffer to use.
• Parser::tokens: allows you to retrieve the token buffer from a parser after you're done with it.

[alamb]

Thank you for the contribution @0rphon -- this PR looks pretty close to me

I had some naming sugestions

Also, I think we should add a test of this API in tests/.. so that a future refactor doesn't accidentally break this API without also having to explicitly change a test

[alamb]

What do you think about calling this into_buffer like:

Suggested change

	pub fn tokenize_with_location_into(
	pub fn tokenize_with_location_into_buffer(

[0rphon]

i changed it to into_buf to match the naming notation of standard library like Read::read_buf or BufRead::fill_buf. let me know if youd rather it be into_buffer though!

[coveralls]

Pull Request Test Coverage Report for Build 7616338243

Warning: This coverage report may be inaccurate.

We've detected an issue with your CI configuration that might affect the accuracy of this pull request's coverage report.
To ensure accuracy in future PRs, please see these guidelines.
A quick fix for this PR: rebase it; your next report should be accurate.

• 0 of 0 changed or added relevant lines in 0 files are covered.
• No unchanged relevant lines lost coverage.
• Overall coverage increased (+0.01%) to 87.868%

---

Totals
Change from base Build 7527748620:	0.01%
Covered Lines:	18889
Relevant Lines:	21497

---

💛 - Coveralls

[trungda]

Just for my education, what do we usually do when we make breaking change in public API like this one? There is a behavior change here right?

[alamb]

I am not sure why this is a breaking change in the sense of "programs that used to compile against sqlparser will still compile against sqlparser after this change". Perhaps I am missing something but I think this PR just adds a new API

In terms of breaking public API changes, for this crate I normally put a note in the changelog

[0rphon]

This PR shouldnt contain any breaking changes to the API. If you looked at the tokenize_with_location_info_buf method that's being called here, its functionally equivalent to the old code. I just separated it out so users can supply their own buffer if desired

[trungda]

Thank you! I misread. I thought that in case of failure, we would still return a partially tokenized vector but actually we are still returning an Err.

[0rphon]

No worries! The new method tokenize_with_location_into_buf will actually return a partial vec of successfully parsed tokens. I specifically needed this behavior for a project im working on that parses a single SQL statement followed by a lot of random unparseable garbage. Then i needed the into_tokens API so i could figure out the length of the parsed statement using Parser::index and a custom token_length function i made. Once i was done with the fork though i realized my changes actually worked perfectly as a way to reuse buffers so i figured id PR it as a little optimization.

[trungda]

Thank @0rphon I am curious how you figure out the length of the parsed statement as I also need that for a separate issue. I think I'll want byte-length for a precise calculation but it's non-trivial to calculate the byte length for all variants of Tokens.

[0rphon]

I just based it off the formatting you use in impl Display for Token. Its a little hacky and definitely not accurate in every scenario, hence why i didn't add it to this PR. I was just trying to determine if a text blob contains an SQL statement, so i just needed an approximate len so i could display a rough output of the matched string.

fn token_len(t: &TokenWithLocation) -> usize {
    match &t.token {
        Token::EOF => 3,
        Token::Word(w) => w.value.len() + w.quote_style.map(|_| 2).unwrap_or_default(),
        Token::Number(n, l) => n.len() + *l as usize,
        Token::Char(c) if c.is_ascii() => 1,
        // todo is this correct?
        Token::Char(_) => 4,
        Token::SingleQuotedString(s) => s.len() + 2,
        Token::DoubleQuotedString(s) => s.len() + 2,
        Token::DollarQuotedString(s) => {
            s.value.len() + s.tag.as_ref().map(|t| t.len() * 2).unwrap_or_default() + 4
        }
        Token::NationalStringLiteral(s) => s.len() + 3,
        Token::EscapedStringLiteral(s) => s.len() + 3,
        Token::HexStringLiteral(s) => s.len() + 3,
        Token::SingleQuotedByteStringLiteral(s) => s.len() + 3,
        Token::DoubleQuotedByteStringLiteral(s) => s.len() + 3,
        Token::RawStringLiteral(s) => s.len() + 3,
        Token::Comma => 1,
        Token::Whitespace(Whitespace::Space) => 1,
        Token::Whitespace(Whitespace::Newline) => 1,
        Token::Whitespace(Whitespace::Tab) => 1,
        Token::Whitespace(Whitespace::SingleLineComment { comment, prefix }) => {
            comment.len() + prefix.len()
        }
        Token::Whitespace(Whitespace::MultiLineComment(s)) => s.len() + 4,
        Token::DoubleEq => 2,
        Token::Spaceship => 3,
        Token::Eq => 1,
        Token::Neq => 2,
        Token::Lt => 1,
        Token::Gt => 1,
        Token::LtEq => 2,
        Token::GtEq => 2,
        Token::Plus => 1,
        Token::Minus => 1,
        Token::Mul => 1,
        Token::Div => 1,
        Token::DuckIntDiv => 2,
        Token::StringConcat => 2,
        Token::Mod => 1,
        Token::LParen => 1,
        Token::RParen => 1,
        Token::Period => 1,
        Token::Colon => 1,
        Token::DoubleColon => 2,
        Token::DuckAssignment => 2,
        Token::SemiColon => 1,
        Token::Backslash => 1,
        Token::LBracket => 1,
        Token::RBracket => 1,
        Token::Ampersand => 1,
        Token::Caret => 1,
        Token::Pipe => 1,
        Token::LBrace => 1,
        Token::RBrace => 1,
        Token::RArrow => 2,
        Token::Sharp => 1,
        Token::ExclamationMark => 1,
        Token::DoubleExclamationMark => 2,
        Token::Tilde => 1,
        Token::TildeAsterisk => 2,
        Token::ExclamationMarkTilde => 2,
        Token::ExclamationMarkTildeAsterisk => 3,
        Token::AtSign => 1,
        Token::ShiftLeft => 2,
        Token::ShiftRight => 2,
        Token::Overlap => 2,
        Token::PGSquareRoot => 2,
        Token::PGCubeRoot => 3,
        Token::Placeholder(s) => s.len(),
        Token::Arrow => 2,
        Token::LongArrow => 3,
        Token::HashArrow => 2,
        Token::HashLongArrow => 3,
        Token::AtArrow => 2,
        Token::ArrowAt => 2,
        Token::HashMinus => 2,
        Token::AtQuestion => 2,
        Token::AtAt => 2,
    }
}

[trungda]

Ah I see. I was thinking of adding the actual byte position of a token in the struct Location to be able to achieve what I want. What do you think?

[0rphon]

that would be a great feature!

[alamb]

👍

[alamb]

Thank you again for this contribution @0rphon and for the review @trungda