Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move TiDB parser to a separate repository #7923

Closed
tiancaiamao opened this issue Oct 16, 2018 · 2 comments
Closed

Move TiDB parser to a separate repository #7923

tiancaiamao opened this issue Oct 16, 2018 · 2 comments
Assignees
Labels
type/enhancement The issue or PR belongs to an enhancement.

Comments

@tiancaiamao
Copy link
Contributor

Feature Request

This is required for #7922

parser

Obviously, the dependency of current parser is ... a total mess

@tiancaiamao tiancaiamao self-assigned this Oct 16, 2018
@tiancaiamao
Copy link
Contributor Author

paresr/ast/model/mysql/types those packages are involved.

The new repo will contain parser/ast packages, for sure.
The mysql package is small enough and has no dependency, so it could be put to the parser repo, too.

The model package contains DBInfo TableInfo CIStr and so on, dependent by ast, it can be move into ast and use the Go1.9 type alias feature:

type DBInfo = ast.DBInfo

The types package may be a trouble.

@tiancaiamao
Copy link
Contributor Author

The types package contains a lot of thing, I don't want it to be put in the parser repository.
Let's consider how to decompose the ast package dependency from types.

Most struct definitions in the ast package are abstract, such as ExprNode Node StmtNode, no problem for them.
The concrete struct definitions used in the ast of package are types.FieldType types.Datum

types.Datum introduce a lot of thing, such as Time Decimal Json ...
Based on the observation that the only place using types.Datum is ValueExpr, if ValueExpr is an abstract one, ast do not necessarily depends on types.Datum. Let some package provide a NewValueExpr function, and make the ValueExpr an interface, the problem could be solved.

Handle types.FieldType is simple, it doesn't have many dependency, move it to the ast package and declare type FieldType = ast.FieldType in the types package would work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/enhancement The issue or PR belongs to an enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants