Split parser into multiple files #351
Conversation
Pull Request Test Coverage Report for Build 1277047061
💛 - Coveralls |
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
Signed-off-by: koushiro <[email protected]>
|
@alamb PTAL |
|
Checking this one out |
|
FWIW I did do a spot check and try this code out with DataFusion and DataFusion's tests still pass |
| } | ||
|
|
||
| // Parse a tab separated values in COPY payload, used in a `COPY` statement | ||
| pub(super) fn parse_tab_values(&mut self) -> Vec<Option<String>> { |
There was a problem hiding this comment.
why is this method moved to dml module by the parse_copy method is left in the root mod?
| @@ -2225,6 +1654,7 @@ impl<'a> Parser<'a> { | |||
| } | |||
| } | |||
|
|
|||
| /// Parse a SQL `DELETE` statement | |||
| pub fn parse_delete(&mut self) -> Result<Statement, ParserError> { | |||
There was a problem hiding this comment.
shouldn't this go into dql as well?
|
I also did a spot check, mostly looks good to me. Couple good improvements on the doc string as well 👍 I have to admit though, personally I think refactoring code into mods makes sense when they are mostly decoupled, which doesn't seem like the case here. I actually find it easier to navigate and search the code base when coupled code are in the same file. That said, it's not a strong no from me. |
|
Sadly I don't have the time to help drive projects such as this. Unless one of the other maintainers wants to do so, PRs like this are not likely to be merged. I am sorry I don't have the time and I am sorry if your time was wasted. As the current state of affairs might not have been clear in the project's README. I have added #416 to try and help |
Pull Request Test Coverage Report for Build 1226236288Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
part of #344
I dropped the commit
Differentiate methodsof #344 to make it easier to be reviewed.visibility changes (no breaking changes):
fn parse_prepare==>pub fn parse_preparefn parse_execute==>pub fn parse_executefn parse_deallocate==>pub fn parse_deallocate/cc @alamb