Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Add _all_ date function aliases to parser (extended)#1982

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
paulrutter wants to merge11 commits intoAlaSQL:develop
base:develop
Choose a base branch
Loading
fromblueconic:feature/1979-add-date-function-aliases-to-parser-extended

Conversation

@paulrutter
Copy link
Contributor

@paulrutterpaulrutter commentedOct 22, 2024•
edited
Loading

Based on#1980, this PR attempts to add all relevant date functions to the parser (and thus validate the query for being valid).

  • NOW
  • GETDATE
  • CURRENT_DATE()
  • ADDDATE(args)
  • SUBDATE(args)
  • DAY(date)
  • DAYOFMONTH(date)
  • DAYOFWEEK(date)
  • DATE_ADD(args)
  • DATE_SUB(args)
  • SUBDATE(args)
  • MINUTE(date)
  • SECOND(date)
  • MONTH(date)
  • YEAR(date)
  • WEEK(date)
  • WEEKDAY(date)

Additional: the interval values that can be specified are now also part of the parser:

    MICROSECOND    | MILLISECOND    | SECOND    | MINUTE    | HOUR    | DAY    | DAYOFWEEK    | DAYOFYEAR    | WEEK    | MONTH    | QUARTER    | YEAR

This makes some keywords now reserved keywords, like usingmonth as column alias for example.
This has been adjusted in this PR.

Do note thatDATE() isnot added to the parser, as its use is very ambigious in AlaSQL.

There are new unit tests that verify the changes made.

mathiasrw reacted with hooray emoji
Add the following functions to the parser:- NOW- GETDATEAlso add `CURRENT_TIMESTAMP()` to the parser (with parenthesis).- Adjust failing test as `now` is a reserved keyword. It was already, it was just not validated.- Add test
- Add all other AlaSQL date functions and aliases to the parser- Adjust unit tests, as month is now a reserved keyword- Explicitly didn't add the "DATE()" function, as it's use is very ambiguous in AlaSQL -> as column alias, as column type, as JS expression.- Added IntervalLiteral which contains all possible values to use when specifying an interval expression
- Use Expression instead of ExprList to restrict the number of arguments to the methods
- Extend unit test with non existing interval- DAYOFYEAR is alias for DAY
- Compact the expression with an OR statement to reduce duplicate code
@mathiasrw
Copy link
Member

I really like this, but im a bit sad to break lots of current implementation by makingyear,month andday (andweek?) reserved words. But I also like it for the consistency.

Over the Christmas break I will see if I can make this work so its only a protected keyword as the first parameter in the date functions, and other places we look for the name plus a(

paulrutter reacted with rocket emoji

@mathiasrw
Copy link
Member

Still working on this. Neck deep in the jungle of JISON.

Copy link
Member

@mathiasrwmathiasrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I got a few questions

Copy link
Member

@mathiasrwmathiasrw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I got a few questions

@paulrutter
Copy link
ContributorAuthor

@mathiasrw i processed your comments and made some changes.

mathiasrw reacted with hooray emoji

Copy link
Member

@mathiasrwmathiasrw left a comment•
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I should have been more clear.

I cant find documentation thatdayofyear anddayofmonth is a valid unit forinterval. I have a feeling those two should not be part of interval literals. What are you seeing that I am not seeing?

@mathiasrw
Copy link
Member

Ill pick up the rest and work on it not being breaking.

paulrutter reacted with hooray emoji

@Charlesnorris509
Copy link

Code Quality Improvements:
Refactor repetitive code patterns identified in the commits to improve code maintainability and readability.
Address the feedback from the pull request comments to ensure code consistency and correctness.

mathiasrw reacted with thumbs down emoji

@mathiasrwmathiasrw self-assigned thisJan 14, 2025
@paulrutter
Copy link
ContributorAuthor

paulrutter commentedJan 14, 2025•
edited
Loading

@mathiasrw what does the PR approval mean? Did you make progress on resolving#1982 (comment)?
I resolved the merge conflicts and regenerated the parser.

mathiasrw reacted with heart emoji

…to-parser-extended# Conflicts:#src/alasqlparser.jison#src/alasqlparser.js
@mathiasrw
Copy link
Member

Well, it means that im subtracting my comments to change because I am working on it locally. I think I got the last part nailed this morning. Ill confirm things over the next few days.

paulrutter reacted with rocket emoji

@paulrutter
Copy link
ContributorAuthor

Sounds great 👍

@mathiasrw
Copy link
Member

omg. Its already end of feb. Sorry things have been hectic.

paulrutter reacted with thumbs up emoji

@paulrutter
Copy link
ContributorAuthor

No problem, take your time 👍

Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

@mathiasrwmathiasrwmathiasrw approved these changes

Assignees

@mathiasrwmathiasrw

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@paulrutter@mathiasrw@Charlesnorris509
[8]ページ先頭
©2009-2025 Movatter.jp