Simple custom lexical precedence in PostgreSQL dialect#1379
Simple custom lexical precedence in PostgreSQL dialect#1379alamb merged 1 commit intoapache:mainfrom
Conversation
Pull Request Test Coverage Report for Build 10387878575Details
💛 - Coveralls |
There was a problem hiding this comment.
Thank you @samuelcolvin -- this looks like an improvement to me. Thank you for the follow up
Since we haven't released the changes in #1360 to crates.io yet, this isn't an API change
I'll wait a day before merging this in case @lovasoa would like to review or comment on this PR as well
Update: I see @lovasoa has already approved it. So let's do this 🚀
| fn prec_unary_not(&self) -> u8 { | ||
| UNARY_NOT_PREC | ||
| /// Uses (APPROXIMATELY) <https://www.postgresql.org/docs/7.0/operators.htm#AEN2026> as a reference | ||
| fn prec_value(&self, prec: Precedence) -> u8 { |
There was a problem hiding this comment.
I think this is a good idea.
My only potential concern with this approach is that it could be potentially slower, as it now has many more function calls, however I don't actually think we have a sqlparser benchmark so at this point so trying to make such a change would likely be a pre-mature optimization.
If anyone needs additional performance, we can do that as follow on PRs
|
|
||
| fn prec_unary_not(&self) -> u8 { | ||
| NOT_PREC | ||
| fn prec_value(&self, prec: Precedence) -> u8 { |
|
thanks a lot. |
|
@alamb if we care about performance, we should stop using dynamic dispatch and make the parser generic around the dialect, with that we could make lots of these methods That would of course be a big change to the public API, but worthwhile IMHO. |
I agree with both points |
|
In my use case (SQLPage), SQL parsing overhead is negligible. Have you ever been bottlenecked by sql parsing? |
No. There is more discussion here as well: #1381 (comment) |
4 participants
As suggest mentioned by @lovasoa in #1360 (comment) the previous approach had unnecessary duplication, this should reduce the duplication of logic while still allowing complete freedom for how dialects decide precedence.
This doesn't change the logic for
PostgreSqlDialectexcept thatToken::DuckIntDivwill now returnMulDivModOpprecedence, notUNKOWN.