iffyio commented on code in PR #1707:
URL: 
https://github.com/apache/datafusion-sqlparser-rs/pull/1707#discussion_r1950766697


##########
src/parser/mod.rs:
##########
@@ -13337,6 +13376,42 @@ impl<'a> Parser<'a> {
         })
     }
 
+    /// Parse an expression, optionally followed by ASC or DESC (used in ORDER 
BY)
+    pub fn parse_create_index_expr<OPS: OperatorClass>(
+        &mut self,
+    ) -> Result<IndexColumn, ParserError> {
+        let expr = self.parse_expr()?;
+
+        let operator_class: Option<OPS> = 
self.parse_one_of_keywords(OPS::KEYWORDS).map(Into::into);

Review Comment:
   I think its fine to allow downstream crates to validate that the operator 
class is compatible with the specified index, that expectation is already 
present when consuming an AST from the parser, see [syntax vs semantics 
section](https://github.com/apache/datafusion-sqlparser-rs?tab=readme-ov-file#syntax-vs-semantics)
 for the parser.
   
   I think in this case there is a lot of code introduced to power this detail 
such that its not worth the accompanying complexity.
   
   Representing it opaquely also makes the parser forwards compatible with with 
postgres going forward. admittedly this is a minor pro but it could be 
desirable given that there's seems to be quite a few of classes



-- 
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: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to