Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose a method for mutating Parser::index #1593

Open
niebayes opened this issue Dec 12, 2024 · 3 comments
Open

Expose a method for mutating Parser::index #1593

niebayes opened this issue Dec 12, 2024 · 3 comments

Comments

@niebayes
Copy link

We have designed an SQL parser using sqlparser's Parser, and we wish to implement two helper methods for consuming tokens—similar to Parser::consume_token and Parser::consume_tokens—but with the ability to accept string inputs and perform case-insensitive matching.

However, since the Parser does not provide a method to mutate the Parser::index, we are unable to correctly implement our own consume_tokens because we need to revert the parser state if consumption fails mid-way.

We propose adding and exposing a new method in sqlparser's Parser, such as set_index or index_mut, to allow users to mutate the index.

The expected implementation of our consume_token and consume_tokens:

/// Consumes the next token if it matches the expected token, otherwise return false.
///
/// Note, the matching is not case sensitive.
fn consume_token(&mut self, expected: &str) -> bool {
    if self.parser.peek_token().to_string().to_uppercase() == *expected.to_uppercase() {
        self.parser.next_token();
        true
    } else {
        false
    }
}

/// If the current and subsequent tokens exactly match the `tokens` sequence, consume them and returns true.
/// Otherwise, no tokens are consumed and returns false
///
/// Note, the matching is not case sensitive.
fn consume_tokens(&mut self, tokens: &[&str]) -> bool {
    let index = self.parser.index();
    for token in tokens {
        if !self.consume_token(*token) {
            // Or: `*self.parser.index_mut() = index;`
            self.parser.set_index(index);
            return false;
        }
    }
    true
}
@niebayes
Copy link
Author

If this makes sense, I'd like to file a PR.

@alamb
Copy link
Contributor

alamb commented Dec 12, 2024

We propose adding and exposing a new method in sqlparser's Parser, such as set_index or index_mut, to allow users to mutate the index.

I think this makes sense to me. Thank you @niebayes

When filing your PR / API can you please add tests (maybe ideally an example in the docstrings) that shows how to use the API / any caveats with it?

cc @iffyio

@niebayes
Copy link
Author

@alamb Sure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants