Move Usage of Default Timezone From Parser to Evaluator

[lziq]

Issue #447

Solution Description:

Originally, the Time in AST uses tz_minutes to decide whether the time has a timezone offset. Null value represents TIME WITHOUT TIME ZONE, while integer value means the timezone offset in minutes for TIME WITH TIME ZONE.

However, this is not enough to distinguish the case of TIME WITH TIME ZONE without explicitly specifying the timezone offset. Thus, we decided to add a new attribute with_time_zone to differentiate 3 cases:

1. If with_time_zone is false, it means TIME WITHOUT TIME ZONE. In this case, tz_minutes should also be null.
2. If with_time_zone is true and tz_minutes is null, it means TIME WITH TIME ZONE without explicitly specifying the timezone offset.
3. If with_time_zone is true and tz_minutes is integer, it means TIME WITH TIME ZONE with a explicitly specified timezone offset.

Changes Details:

1. Added the new attribute with_time_zone to Time in domain-types, serialization and deserializationof AST, and AST.
2. Removed the code to add default time zone in sqlParser, and afterwards refactored the block of code.
3. Added the code to add default time zone in EvaluatingCompilor.
4. Modified test cases.

[abhikuhikar]

Overall looks good. Some minor changes required. Great job.

[codecov-commenter]

Codecov Report

Merging #448 (14b712c) into main (68e9f61) will increase coverage by 0.03%.
The diff coverage is 100.00%.

@@             Coverage Diff              @@
##               main     #448      +/-   ##
============================================
+ Coverage     82.31%   82.35%   +0.03%     
  Complexity     1394     1394              
============================================
  Files           171      171              
  Lines         10723    10717       -6     
  Branches       1769     1766       -3     
============================================
- Hits           8827     8826       -1     
+ Misses         1353     1350       -3     
+ Partials        543      541       -2     

Flag	Coverage Δ
CLI	18.18% <ø> (ø)
EXAMPLES	74.85% <ø> (ø)
LANG	84.85% <100.00%> (+0.04%)	⬆️
PTS	∅ <ø> (∅)
TEST_SCRIPT	79.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ng/src/org/partiql/lang/ast/ExprNodeToStatement.kt	93.77% <100.00%> (+0.02%)	⬆️
...ng/src/org/partiql/lang/ast/StatementToExprNode.kt	97.35% <100.00%> (+<0.01%)	⬆️
lang/src/org/partiql/lang/ast/ast.kt	89.13% <100.00%> (+0.02%)	⬆️
...src/org/partiql/lang/ast/passes/AstRewriterBase.kt	100.00% <100.00%> (ø)
...ng/src/org/partiql/lang/eval/EvaluatingCompiler.kt	84.22% <100.00%> (ø)
lang/src/org/partiql/lang/syntax/SqlParser.kt	79.72% <100.00%> (+0.02%)	⬆️
...ng/src/org/partiql/lang/util/IonValueExtensions.kt	44.53% <0.00%> (-0.35%)	⬇️
lang/src/org/partiql/lang/domains/util.kt	71.42% <0.00%> (+1.42%)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68e9f61...14b712c. Read the comment docs.

[abhikuhikar]

Ship it.

[abhikuhikar]

Can you change the commit message/description to something like "Removes usage of DEFAULT_TIME_ZONE from parser to evaluator."?
The goal here is to get the gist of the commit from a commit message.
Also make sure you squash-merge.

[alancai98]

Looks good! Thanks for including a thorough summary of changes and linking the issue. Only have one very minor nit.

Other general comments:

• +1 to what @abhikuhikar said about changing changing the commit message to something more descriptive. It would also be good to update this PR's title, so it's clear what the PR fixes without needing to click the issue.
• Try not to include special characters in the branch name (like '#'). Checking out the branch locally can cause problems (depending on the shell version):

❯ git checkout fix-issue#447
zsh: no matches found: fix-issue#447

❯ git checkout 'fix-issue#447'   # notice I had to add single quotes
Switched to branch 'fix-issue#447'
Your branch is up to date with 'origin/fix-issue#447'.