Repository navigation
Suggest splitting parser.rs into smaller mod files #944
Description
Activity
The overall directory structure after sorting is as follows:
./src/parser/ |-- alter.rs |-- analyze.rs |-- assert.rs |-- begin.rs |-- cache.rs |-- close.rs |-- commit.rs |-- copy.rs |-- create.rs |-- deallocate.rs |-- declare.rs |-- delete.rs |-- describe.rs |-- discard.rs |-- drop.rs |-- execute.rs |-- explain.rs |-- fetch.rs |-- grant.rs |-- insert.rs |-- kill.rs |-- merge.rs |-- mod.rs |-- msck.rs |-- prepare.rs |-- revoke.rs |-- rollback.rs |-- savepoint.rs |-- select.rs |-- set.rs |-- show.rs |-- start.rs |-- truncate.rs |-- uncache.rs |-- update.rs |-- use.rs `-- utils.rs
This basic plan seems reasonable to me, though it will cause substantial conflicts with anyone who as an outstaning PR 🤔
@alamb If possible, we can break it up into some issues and PRs to step through this refactoring.
Sounds like a good plan for me
Do you have any suggestions for this? Thank you very much. 😊
My bandwidth for this repo is quite low. However, if you make PRs that are easy to review (just moving code around) I think I can review and merge them pretty fast
Reacted by r.4ntix and Augusto Fotino@alamb one question, not 100% related but... What are your thoughts on creating a trait
Parseto implement for the structures/enums, instead of loose functions?This would make it easier to identify where things happen. Sometimes I find myself trying to figure which method parses which structure, as they are all together and, sometimes, with the same return.
What are your thoughts on creating a trait Parse to implement for the structures/enums, instead of loose functions?
I am not quite sure what you are proposing @AugustoFKL -- what would be an example of such a trait?
A structured trait could be a nice idea and I would personally welcome it, but also a monumental refactoring at this time. I think we should start by splitting up into submodules, maybe just before the next release so we have a "frozen repository" moment after having merged the PRs we can manage.
splitting sounds like a great first step
Closing in favour of #2591, which tracks the same split as a series of pure-move PRs before the next release. The structured trait idea will be explored at some point in the future but it will be certainly a major refactoring requiring a non trivial amount of time.
Background
The code of current
parser.rsis too large, it's difficult to read and maintain. I'd suggest splitting it into smaller mod files.Proposal
I have split the
ALTER ROLEcode intoparser/alter.rsin this PR feat: addALTER ROLEsyntax of PostgreSQL and MS SQL Server by r4ntix · Pull Request #942 · sqlparser-rs/sqlparser-rs (github.com)I think we can do a refactoring and split the parser, like this:
I wonder what people would suggest about this?
Implementation Plan
we can break it up into some issues and PRs to step through this refactoring.
DDL
all tasks:
DML
all tasks:
DCL
all tasks:
PostgreSQL/BigQuery/MySQL/SparkSQL/ClickHouse/Hive
all tasks: