Skip to content

Suggest splitting parser.rs into smaller mod files #944

Closed as duplicate
Closed as duplicate
Suggest splitting parser.rs into smaller mod files#944

Description

@r4ntix
r4ntix
opened on Aug 9, 2023

Background

The code of current parser.rs is too large, it's difficult to read and maintain. I'd suggest splitting it into smaller mod files.

Proposal

I have split the ALTER ROLE code into parser/alter.rs in this PR feat: add ALTER ROLE syntax 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:

./src/parser/
|-- alter.rs
|-- create.rs
|-- drop.rs
|-- mod.rs
|-- select.rs
|-- ...
`-- utils.rs

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:

  • ANALYZE
  • ALTER
  • CREATE
  • DECLARE
  • DROP
  • UPDATE
  • TRUNCATE

DML

all tasks:

  • DELETE
  • EXPLAIN
  • FETCH
  • INSERT
  • SELECT
  • UPDATE
  • MERGE
  • MSCK
  • ROLLBACK
  • SET
  • START
  • TRUNCATE

DCL

all tasks:

  • COMMIT
  • DESCRIBE
  • DISCARD
  • GRANT
  • REVOKE
  • SAVEPOINT
  • SHOW
  • USE

PostgreSQL/BigQuery/MySQL/SparkSQL/ClickHouse/Hive

all tasks:

  • ASSERT
  • BEGIN
  • CACHE
  • CLOSE
  • COPY
  • DEALLOCATE
  • EXECUTE
  • KILL
  • PREPARE
  • UNCACHE

Activity

  1. r4ntix commented on Aug 10, 2023

    @r4ntix
    r4ntix
    on Aug 10, 2023
    ContributorAuthor

    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

  2. alamb commented on Aug 17, 2023

    @alamb
    alamb
    on Aug 17, 2023
    Contributor

    This basic plan seems reasonable to me, though it will cause substantial conflicts with anyone who as an outstaning PR 🤔

  3. r4ntix commented on Aug 18, 2023

    @r4ntix
    r4ntix
    on Aug 18, 2023
    ContributorAuthor

    @alamb If possible, we can break it up into some issues and PRs to step through this refactoring.
    Do you have any suggestions for this? Thank you very much. 😊

  4. alamb commented on Aug 18, 2023

    @alamb
    alamb
    on Aug 18, 2023
    Contributor

    @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

  5. AugustoFKL commented on Sep 12, 2023

    @AugustoFKL
    AugustoFKL
    on Sep 12, 2023
    Contributor

    @alamb one question, not 100% related but... What are your thoughts on creating a trait Parse to 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.

  6. alamb commented on Sep 13, 2023

    @alamb
    alamb
    on Sep 13, 2023
    Contributor

    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?

  7. LucaCappelletti94 commented on Sep 26, 2026

    @LucaCappelletti94
    LucaCappelletti94
    on Sep 26, 2026
    Contributor

    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.

  8. alamb commented on Sep 27, 2026

    @alamb
    alamb
    on Sep 27, 2026
    Contributor

    splitting sounds like a great first step

  9. LucaCappelletti94 commented on Sep 28, 2026

    @LucaCappelletti94
    LucaCappelletti94
    on Sep 28, 2026
    Contributor

    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.

  10. LucaCappelletti94
    closed this as a duplicateof Split src/parser/mod.rs and src/ast/mod.rs into smaller modules #2591 on Sep 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions