update LimboError::ParseError(String) to LimboError::ParseError(ParseError) #3639
Conversation
alpaylan
requested review from
jussisaurio,
penberg and
pereman2
as code owners
parser/src/error.rs
Outdated
| /// Table not found in get_collseq_from_expr | ||
| #[error("table not found")] | ||
| TableNotFound, | ||
| /// Column not found in get_collseq_from_expr |
There was a problem hiding this comment.
Why is this specific to get_collseq_from_expr? Should we rename this to NoSuchColumn and return it for all the no such column errors?
There was a problem hiding this comment.
A part of this was that I was scared of breaking any compatibility so I kept every error letter by letter, that's why some errors aren't deduplicated properly I think. It makes sense to me that we merge NoSuchColumn for all no such column errors.
There was a problem hiding this comment.
Got it - I'd say don't worry about it, if the tests pass then we're good. Let's not get those fears get in the way of improving the codebase 👍 we're still 0.x after all, i would be surprised if end users were already relying on specific error messages
There was a problem hiding this comment.
I'll make some more updates to consolidate some errors even more, after that we should be good to merge I think.
2 participants
No description provided.