Skip to content

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

Open
paulrutter wants to merge 11 commits intoAlaSQL:developfrom
blueconic:feature/1979-add-date-function-aliases-to-parser-extended
Open

Add _all_ date function aliases to parser (extended)#1982
paulrutter wants to merge 11 commits intoAlaSQL:developfrom
blueconic:feature/1979-add-date-function-aliases-to-parser-extended

Conversation

@paulrutter
Copy link
Contributor

@paulrutter paulrutter commented Oct 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 using month as column alias for example.
This has been adjusted in this PR.

Do note that DATE() is not added to the parser, as its use is very ambigious in AlaSQL.

There are new unit tests that verify the changes made.

@paulrutter
1979
677dabd 
Add the following functions to the parser:
- NOW
- GETDATE

Also 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
@paulrutter
1979-add-date-function-aliases-to-parser
444e8b1 
- 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
@paulrutter paulrutter mentioned this pull request Oct 22, 2024
Closed
@paulrutter
1979-add-date-function-aliases-to-parser
620426d 
- Use Expression instead of ExprList to restrict the number of arguments to the methods
@paulrutter
1979-add-date-function-aliases-to-parser
51bf815 
- Extend unit test with non existing interval
- DAYOFYEAR is alias for DAY
@paulrutter
1979-add-date-function-aliases-to-parser
30813b9 
- Compact the expression with an OR statement to reduce duplicate code
@paulrutter paulrutter mentioned this pull request Nov 1, 2024
Closed
@paulrutter paulrutter mentioned this pull request Dec 10, 2024
Open
@mathiasrw
Copy link
Member

mathiasrw commented Dec 15, 2024

I really like this, but im a bit sad to break lots of current implementation by making year, month and day (and week?) 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 (

@mathiasrw
Copy link
Member

mathiasrw commented Dec 19, 2024

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

mathiasrw
mathiasrw requested changes Dec 20, 2024
View reviewed changes
Copy link
Member

@mathiasrw mathiasrw 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

mathiasrw
mathiasrw reviewed Dec 20, 2024
View reviewed changes
Copy link
Member

@mathiasrw mathiasrw 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
Contributor Author

paulrutter commented Dec 20, 2024

@mathiasrw i processed your comments and made some changes.

mathiasrw
mathiasrw reviewed Dec 21, 2024
View reviewed changes
Copy link
Member

@mathiasrw mathiasrw 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 that dayofyear and dayofmonth is a valid unit for interval. 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

mathiasrw commented Dec 29, 2024

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

@Charlesnorris509
Copy link

Charlesnorris509 commented Jan 14, 2025

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 mathiasrw self-assigned this Jan 14, 2025
@paulrutter
Copy link
Contributor Author

paulrutter commented Jan 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.

@paulrutter
Merge branch 'develop_2' into feature/1979-add-date-function-aliases-…
741599f 
…to-parser-extended

# Conflicts:
#	src/alasqlparser.jison
#	src/alasqlparser.js
@mathiasrw
Copy link
Member

mathiasrw commented Jan 15, 2025

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
Copy link
Contributor Author

paulrutter commented Jan 15, 2025

Sounds great 👍

@mathiasrw
Copy link
Member

mathiasrw commented Feb 28, 2025

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

@paulrutter
Copy link
Contributor Author

paulrutter commented Feb 28, 2025

No problem, take your time 👍

@mathiasrw mathiasrw added this to the Next major version bump milestone Dec 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mathiasrw mathiasrw mathiasrw approved these changes

Assignees

@mathiasrw mathiasrw

Labels

None yet

Projects

None yet

Milestone

Next major version bump

Development

Successfully merging this pull request may close these issues.

3 participants

@paulrutter @mathiasrw @Charlesnorris509