-
Notifications
You must be signed in to change notification settings - Fork 3
[Refactor] Improve query structure nodes #88
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
Conversation
Code Changes for the Parser
@colinthebomb1 Could you take a look at the new code I added? Thanks a lot! |
core/ast/node.py
Outdated
| class OrderByNode(Node): | ||
| """ORDER BY clause node""" | ||
| def __init__(self, _items: List['Node'], **kwargs): | ||
| def __init__(self, _items: List['Node'], _sort: Optional[str] = None, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above. Since sort approach (DESC, ASC) are two SQL keywords, should we also use enum?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
| super().__init__(NodeType.HAVING, children=_predicates, **kwargs) | ||
|
|
||
|
|
||
| class OrderByItemNode(Node): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the difference between OrderByItemNode and OrderByNode?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OrdrByItemNode is for single items in ORDER BY clauses like
OrderByItemNode(ColumnNode("salary"), _sort = SortOrder.DESC).
OrderByNode is used to build full ORDER BY clauses formed by multiple OrderByItemNode like
OrderByNode([
OrderByItemNode(ColumnNode("salary"), _sort=SortOrder.DESC),
OrderByItemNode(ColumnNode("age"), _sort=SortOrder.ASC)
])
|
@HazelYuAhiru, other than the JoinType enum that Qiushi pointed out, your changes LGTM! I added the eq and hash for the nodes I was working on, too. |
Overview:
This PR improves the node ast
Code Changes:
FunctionNode, allows for cases likeSELECT COUNT(*) as emp_countSelect, From, Where, and Having nodes, allowing for order of items in clauses to be maintainedsortparameter inOrderBynode and createdOrderByItemNodeto allow more functionality in ORDER BY statements