feat(parser): add support for QUARTER duration alias and related tests#35268
feat(parser): add support for QUARTER duration alias and related tests#35268guanshengliang merged 6 commits into3.0from
Conversation
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.
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.
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.
Co-authored-by: Copilot <copilot@github.com>
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
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
dapan1121
left a comment
There was a problem hiding this comment.
[MAJOR] source/libs/parser/src/parTranslater.c:10325 — checkEvery 未处理大写 N/Y
问题描述:
// 新代码
static int32_t checkEvery(STranslateContext* pCxt, SValueNode* pInterval) {
char unit = pInterval->unit;
if (unit == 'n' || unit == 'y') { // ← 仅检查小写
return generateSyntaxErrMsgExt(..., "Unsupported time unit in EVERY clause");
}在 parseNatualDuration 中,q/Q 被规范化为 'n'(小写),故 translateExpr 执行后
pInterval->unit 对 1q/1Q 均为 'n',checkEvery 能正确拒绝。
但对于 1N/1Y,parseNatualDuration 不做规范化(IS_CALENDAR_TIME_DURATION 返回成功
但 *unit 仍为 'N'/'Y'),所以 pInterval->unit 为 'N' 或 'Y',与 'n'/'y'
不匹配,会静默放行。
影响:本 PR 在 test_query_interp_fill.py 中新增了 every(1N) / every(1Y) 的
拒绝测试,若 N/Y 未被上游其他路径规范化为小写,这两个用例将 CI 失败。
原因分析:这是一个在 checkEvery 重构过程中遗漏的边界情况。旧代码从 literal 字符串
取末位字符同样只比较小写,所以此问题是预存 Bug。但本 PR 新增了依赖该行为的测试,会将
隐患暴露为 CI 失败。
建议修改:使用已有宏替代硬编码,既修复问题又保持未来可扩展性:
static int32_t checkEvery(STranslateContext* pCxt, SValueNode* pInterval) {
char unit = pInterval->unit;
if (IS_CALENDAR_TIME_DURATION(unit)) {
return generateSyntaxErrMsgExt(&pCxt->msgBuf, TSDB_CODE_PAR_WRONG_VALUE_TYPE,
"Unsupported time unit in EVERY clause");
}若上游已有其他路径将 N/Y 规范化为小写,请在 checkEvery 附近加注释说明依赖关系,
并确认已有 CI 覆盖。
[MINOR] source/common/test/ttimeQuarterParseTest.cpp:540 — 下溢测试 negativeAllow 参数覆盖不完整
问题描述:
// 上溢测试:negativeAllow=false(正数值路径)
int32_t code = parseNatualDuration(buf, len, &duration, &unit,
TSDB_TIME_PRECISION_MILLI, false);
// 下溢测试:negativeAllow=true(负数值路径)
int32_t code = parseNatualDuration(buf, len, &duration, &unit,
TSDB_TIME_PRECISION_MILLI, true);原因分析:测试注释解释了差异原因,下溢路径需要 negativeAllow=true 才能令 taosStr2Int64
接受负号并执行到溢出检查,逻辑本身正确。但正常业务调用时也可能以 negativeAllow=true
触发正数上溢路径,当前测试未覆盖该组合。
建议修改:补充一个 negativeAllow=true 下的正数上溢测试用例,例如:
TEST_F(QuarterParseTest, OverflowProtectionNegativeAllowTrue) {
char buf[64];
int len = snprintf(buf, sizeof(buf), "%" PRId64 "q", (int64_t)(INT64_MAX / 3 + 1));
int64_t duration = 0;
char unit = 0;
int32_t code = parseNatualDuration(buf, len, &duration, &unit,
TSDB_TIME_PRECISION_MILLI, true);
EXPECT_NE(code, TSDB_CODE_SUCCESS);
}[INFO] stream.md 中 PERIOD 支持 q 缺少验证
问题描述:
文档更新后,PERIOD 的 period_time 参数描述改为链接时间单位表(含 q),隐含 PERIOD
支持季度。但本 PR 无对应的 PERIOD+季度集成测试(如 PERIOD(1q, ...)),且 PERIOD 代码
路径无任何修改。
建议:补充一个 PERIOD(1q) 的正/负用例,或在文档中明确说明 q 通过 3n 规范化
后在 PERIOD 中透明生效(需先确认 PERIOD 内部调用了 parseNatualDuration)。
…on in stream creation
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 20 out of 20 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
source/libs/parser/src/parTranslater.c:10356
translateInterpEvery()reads((SValueNode*)(*pEvery))->datum.iand may return the generic "Unsupported time unit in EVERY clause" error even whentranslateExpr()orcheckEvery()has already failed. This can mask the real translation error (and relies ondatum.ibeing meaningful when translation fails). Consider returning early whencode != TSDB_CODE_SUCCESSbefore inspectingdatum.i/ emitting the interval==0 error.
| */ | ||
|
|
||
| #include <gtest/gtest.h> | ||
| #include <inttypes.h> |
| | `m` | 分钟 | | | ||
| | `h` | 小时 | | | ||
| | `d` | 天 | | | ||
| | `w` | 周(7 天) | | |
There was a problem hiding this comment.
是自然周,但是这个pr没有对自然周的起始进行修改。
计划在新增firstDayOfWeek时调整周的起止点为设置的值,默认情况下与现在兼容。
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.