feat(parser): add support for quarter duration alias and related tests#35268
feat(parser): add support for quarter duration alias and related tests#35268
Conversation
Co-authored-by: Copilot <copilot@github.com>
Tony2h
requested review from
a team,
dapan1121 and
guanshengliang
as code owners
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for the quarter duration unit ('q' or 'Q') by normalizing it to three months ('3n') during natural duration parsing. The changes include updates to the tokenizer, parser logic with overflow protection, and a comprehensive suite of unit and integration tests covering interval boundaries, leap years, and timezone/DST scenarios. Review feedback suggests utilizing the IS_CALENDAR_TIME_DURATION macro to improve consistency and ensure that uppercase calendar units are correctly handled in both absolute and natural duration parsing contexts.
| /* natural month/year/quarter are not allowed in absolute duration */ | ||
| *unit = token[tokenlen - 1]; | ||
| if (*unit == 'n' || *unit == 'y') { | ||
| if (*unit == 'n' || *unit == 'y' || *unit == 'q' || *unit == 'Q') { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| *duration *= 3; | ||
| *unit = 'n'; | ||
| } | ||
| if (*unit == 'n' || *unit == 'y') { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
There was a problem hiding this comment.
Pull request overview
Adds SQL/parser support for a quarter duration alias (q/Q) by normalizing it to 3n (months), and introduces new unit + integration tests to validate interval behavior (including timezone/DST scenarios).
Changes:
- Extend duration tokenization/parsing to accept
q/Qand normalize to3n. - Tighten interval offset validation for calendar intervals (reject offset equal to interval) and adjust
EVERYvalidation flow. - Add C++ unit tests and Python query tests (including CI task wiring) for quarter intervals and timezone/DST behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/ci/cases.task | Adds the new quarter interval/timezone pytest modules to CI execution list. |
| test/cases/09-DataQuerying/01-Select/test_query_quarter_interval.py | New functional tests validating interval(…q) equivalence and negative cases. |
| test/cases/09-DataQuerying/01-Select/test_query_quarter_timezone.py | New timezone/DST coverage for quarter intervals, includes xfail cases documenting a known timezone propagation issue. |
| source/libs/parser/src/parTranslater.c | Rejects calendar offset == interval; adjusts EVERY translation/check order and unit source. |
| source/libs/parser/src/parTokenizer.c | Extends numeric+unit tokenization to include q/Q. |
| source/common/src/ttime.c | Adds q/Q handling: normalize to 3n; reject q/Q in absolute-duration parsing. |
| source/common/test/ttimeQuarterParseTest.cpp | New gtest suite for quarter parsing normalization + rejection in absolute durations. |
| source/common/test/CMakeLists.txt | Builds/registers the new ttimeQuarterParseTest target. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <copilot@github.com>
…to a single test file for improved organization and maintainability. Co-authored-by: Copilot <copilot@github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new calendar-duration alias for quarters (q/Q → 3n) to the SQL parser/time parsing layer, and introduces both unit tests and query-level regression tests to validate quarter window behavior (including offsets and timezones).
Changes:
- Extend duration tokenization and time parsing to recognize
q/Qand normalize it to months (n) for natural durations, while rejecting it in absolute-duration contexts. - Tighten interval offset validation for calendar units by rejecting
offset == interval(consistent with fixed-duration behavior). - Add new C++ unit tests and Python query tests (plus CI task registration) covering normalization, boundary alignment, offsets, and DST/timezone behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/ci/cases.task | Registers the new quarter query test in CI task list. |
| test/cases/09-DataQuerying/01-Select/test_query_quarter.py | Adds comprehensive interval(…q) query tests, including negative cases and timezone/DST coverage. |
| source/libs/parser/src/parTranslater.c | Adjusts interval offset validation (>=), and reorders EVERY translation/checking to use parsed unit. |
| source/libs/parser/src/parTokenizer.c | Updates tokenizer to accept q/Q in duration-like literals (e.g., 1q). |
| source/common/test/ttimeQuarterParseTest.cpp | Adds C++ unit tests for q/Q normalization and rejection in absolute durations. |
| source/common/test/CMakeLists.txt | Builds/registers the new ttimeQuarterParseTest executable. |
| source/common/src/ttime.c | Implements q/Q normalization (*3, unit → n) with overflow checks; rejects q/Q in absolute durations. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| global _quarter_db_initialized | ||
|
|
||
| if not _quarter_db_initialized: | ||
| tdSql.execute(f"drop database if exists {QUARTER_TEST_DB}") | ||
| tdSql.execute(f"create database {QUARTER_TEST_DB} precision 'ms'") | ||
| _quarter_db_initialized = True | ||
|
|
||
| tdSql.execute(f"use {QUARTER_TEST_DB}") |
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.