novartole commented on code in PR #2184:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/2184#discussion_r2733800871


##########
src/tokenizer.rs:
##########
@@ -917,6 +937,23 @@ impl<'a> Tokenizer<'a> {
         self
     }
 
+    /// Return an iterator over tokens
+    pub fn iter(&mut self) -> TokenWithLocationIter<'a, '_> {

Review Comment:
   1. What is the purpose of making it public?
   2. `iter` doesn't reflect what the method does and sounds too general.
   3. `TokenWithLocationIter` returns `TokenWithSpan` rather instead something 
with `Location`.
   
   How about giving it a more specific name and making it private?
   
   ```suggestion
       fn token_with_span_iter(&mut self) -> TokenWithSpanIter<'a, '_> {
   ```



##########
src/tokenizer.rs:
##########
@@ -2299,6 +2325,34 @@ impl<'a> Tokenizer<'a> {
     }
 }
 
+/// Iterator over tokens.
+pub struct TokenWithLocationIter<'a, 'b> {
+    state: State<'a>,
+    location: Location,
+    tokenizer: &'b mut Tokenizer<'a>,
+    prev_token_kind: Option<PrevTokenKind>,
+}
+
+impl Iterator for TokenWithLocationIter<'_, '_> {
+    type Item = Result<TokenWithSpan, TokenizerError>;
+
+    fn next(&mut self) -> Option<Self::Item> {
+        let token = match self
+            .tokenizer
+            .next_token(&mut self.state, self.prev_token_kind)
+        {
+            Err(err) => return Some(Err(err)),
+            Ok(None) => return None,
+            Ok(Some(token)) => token,

Review Comment:
   BTW, `transpose` seems to play well here:
   
   ```suggestion
               .next_token(&mut self.state, self.prev_token_kind)
               .transpose()?
           {
               Err(err) => return Some(Err(err)),
               Ok(token) => token,
   ```



##########
src/tokenizer.rs:
##########
@@ -2577,6 +2631,39 @@ mod tests {
         compare(expected, tokens);
     }
 
+    #[test]
+    fn tokenize_iterator_map() {
+        let sql = String::from("SELECT $1");
+        let dialect = GenericDialect {};
+        let mut param_num = 1;
+
+        let tokens = Tokenizer::new(&dialect, &sql)
+            .iter()
+            .map(|token| {
+                let token = token?;
+                Ok(match token.token {
+                    Token::Placeholder(n) => Token::Placeholder(if n == "?" {
+                        let ret = format!("${}", param_num);
+                        param_num += 1;
+                        ret
+                    } else {
+                        n
+                    }),
+                    _ => token.token,
+                })
+            })
+            .collect::<Result<Vec<_>, TokenizerError>>()
+            .unwrap();

Review Comment:
   Manual parsing doesn't seem to be the source of truth. I guess you mean the 
following:
   ```suggestion
           let tokens = Tokenizer::new(&dialect, &sql).tokenize().unwrap();
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to