From ec643ceecd6979ea95d98585a0fc9c734f0d23c8 Mon Sep 17 00:00:00 2001 From: Xiaoyu Wang Date: Tue, 12 Apr 2022 16:12:45 +0800 Subject: [PATCH] fix: is_null expression returned an incorrect result --- include/util/taoserror.h | 2 + source/libs/parser/src/parTranslater.c | 50 +++++++++++++++++++---- source/libs/parser/src/parUtil.c | 30 ++++++++------ source/libs/parser/test/parserAstTest.cpp | 10 +++++ 4 files changed, 72 insertions(+), 20 deletions(-) diff --git a/include/util/taoserror.h b/include/util/taoserror.h index 04baa3c09d..60535e2f49 100644 --- a/include/util/taoserror.h +++ b/include/util/taoserror.h @@ -597,6 +597,8 @@ int32_t* taosGetErrno(); #define TSDB_CODE_PAR_INVALID_ROLLUP_OPTION TAOS_DEF_ERROR_CODE(0, 0x2622) #define TSDB_CODE_PAR_INVALID_RETENTIONS_OPTION TAOS_DEF_ERROR_CODE(0, 0x2623) #define TSDB_CODE_PAR_GROUPBY_WINDOW_COEXIST TAOS_DEF_ERROR_CODE(0, 0x2624) +#define TSDB_CODE_PAR_INVALID_OPTION_UNIT TAOS_DEF_ERROR_CODE(0, 0x2625) +#define TSDB_CODE_PAR_INVALID_KEEP_UNIT TAOS_DEF_ERROR_CODE(0, 0x2626) //planner #define TSDB_CODE_PLAN_INTERNAL_ERROR TAOS_DEF_ERROR_CODE(0, 0x2700) diff --git a/source/libs/parser/src/parTranslater.c b/source/libs/parser/src/parTranslater.c index 8885373b2b..acf465bfa5 100644 --- a/source/libs/parser/src/parTranslater.c +++ b/source/libs/parser/src/parTranslater.c @@ -21,7 +21,7 @@ #include "parUtil.h" #include "ttime.h" -#define GET_OPTION_VAL(pVal, defaultVal) (NULL == (pVal) ? (defaultVal) : ((SValueNode*)(pVal))->datum.i) +#define GET_OPTION_VAL(pVal, defaultVal) (NULL == (pVal) ? (defaultVal) : getBigintFromValueNode((SValueNode*)(pVal))) typedef struct STranslateContext { SParseContext* pParseCxt; @@ -380,6 +380,7 @@ static EDealRes translateColumn(STranslateContext* pCxt, SColumnNode* pCol) { static EDealRes translateValue(STranslateContext* pCxt, SValueNode* pVal) { uint8_t precision = (NULL != pCxt->pCurrStmt ? pCxt->pCurrStmt->precision : pVal->node.resType.precision); + pVal->node.resType.precision = precision; if (pVal->isDuration) { if (parseNatualDuration(pVal->literal, strlen(pVal->literal), &pVal->datum.i, &pVal->unit, precision) != TSDB_CODE_SUCCESS) { @@ -452,6 +453,9 @@ static EDealRes translateOperator(STranslateContext* pCxt, SOperatorNode* pOp) { } pOp->node.resType.type = TSDB_DATA_TYPE_DOUBLE; pOp->node.resType.bytes = tDataTypes[TSDB_DATA_TYPE_DOUBLE].bytes; + } else { + pOp->node.resType.type = TSDB_DATA_TYPE_BOOL; + pOp->node.resType.bytes = tDataTypes[TSDB_DATA_TYPE_BOOL].bytes; } return DEAL_RES_CONTINUE; } @@ -1041,6 +1045,27 @@ static int32_t translateSelect(STranslateContext* pCxt, SSelectStmt* pSelect) { return code; } +static int64_t getUnitPerMinute(uint8_t precision) { + switch (precision) { + case TSDB_TIME_PRECISION_MILLI: + return MILLISECOND_PER_MINUTE; + case TSDB_TIME_PRECISION_MICRO: + return MILLISECOND_PER_MINUTE * 1000L; + case TSDB_TIME_PRECISION_NANO: + return NANOSECOND_PER_MINUTE; + default: + break; + } + return MILLISECOND_PER_MINUTE; +} + +static int64_t getBigintFromValueNode(SValueNode* pVal) { + if (pVal->isDuration) { + return pVal->datum.i / getUnitPerMinute(pVal->node.resType.precision); + } + return pVal->datum.i; +} + static int32_t buildCreateDbRetentions(const SNodeList* pRetentions, SCreateDbReq* pReq) { if (NULL != pRetentions) { pReq->pRetensions = taosArrayInit(LIST_LENGTH(pRetentions), sizeof(SRetention)); @@ -1098,7 +1123,10 @@ static int32_t checkRangeOption(STranslateContext* pCxt, const char* pName, SVal if (DEAL_RES_ERROR == translateValue(pCxt, pVal)) { return pCxt->errCode; } - int64_t val = pVal->datum.i; + if (pVal->isDuration && (TIME_UNIT_MINUTE != pVal->unit && TIME_UNIT_HOUR != pVal->unit && TIME_UNIT_DAY != pVal->unit)) { + return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_OPTION_UNIT, pName, pVal->unit); + } + int64_t val = getBigintFromValueNode(pVal); if (val < minVal || val > maxVal) { return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_RANGE_OPTION, pName, val, minVal, maxVal); } @@ -1187,9 +1215,18 @@ static int32_t checkKeepOption(STranslateContext* pCxt, SNodeList* pKeep) { } } - int32_t daysToKeep0 = ((SValueNode*)nodesListGetNode(pKeep, 0))->datum.i; - int32_t daysToKeep1 = ((SValueNode*)nodesListGetNode(pKeep, 1))->datum.i; - int32_t daysToKeep2 = ((SValueNode*)nodesListGetNode(pKeep, 2))->datum.i; + SValueNode* pKeep0 = (SValueNode*)nodesListGetNode(pKeep, 0); + SValueNode* pKeep1 = (SValueNode*)nodesListGetNode(pKeep, 1); + SValueNode* pKeep2 = (SValueNode*)nodesListGetNode(pKeep, 2); + if ((pKeep0->isDuration && (TIME_UNIT_MINUTE != pKeep0->unit && TIME_UNIT_HOUR != pKeep0->unit && TIME_UNIT_DAY != pKeep0->unit)) || + (pKeep1->isDuration && (TIME_UNIT_MINUTE != pKeep1->unit && TIME_UNIT_HOUR != pKeep1->unit && TIME_UNIT_DAY != pKeep1->unit)) || + (pKeep2->isDuration && (TIME_UNIT_MINUTE != pKeep2->unit && TIME_UNIT_HOUR != pKeep2->unit && TIME_UNIT_DAY != pKeep2->unit))) { + return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_KEEP_UNIT, pKeep0->unit, pKeep1->unit, pKeep2->unit); + } + + int32_t daysToKeep0 = getBigintFromValueNode(pKeep0); + int32_t daysToKeep1 = getBigintFromValueNode(pKeep1); + int32_t daysToKeep2 = getBigintFromValueNode(pKeep2); if (daysToKeep0 < TSDB_MIN_KEEP || daysToKeep1 < TSDB_MIN_KEEP || daysToKeep2 < TSDB_MIN_KEEP || daysToKeep0 > TSDB_MAX_KEEP || daysToKeep1 > TSDB_MAX_KEEP || daysToKeep2 > TSDB_MAX_KEEP) { return generateDealNodeErrMsg(pCxt, TSDB_CODE_PAR_INVALID_KEEP_VALUE, daysToKeep0, daysToKeep1, daysToKeep2, @@ -1240,8 +1277,7 @@ static int32_t checkDatabaseOptions(STranslateContext* pCxt, SDatabaseOptions* p code = checkRangeOption(pCxt, "compression", pOptions->pCompressionLevel, TSDB_MIN_COMP_LEVEL, TSDB_MAX_COMP_LEVEL); } if (TSDB_CODE_SUCCESS == code) { - code = - checkRangeOption(pCxt, "daysPerFile", pOptions->pDaysPerFile, TSDB_MIN_DAYS_PER_FILE, TSDB_MAX_DAYS_PER_FILE); + code = checkRangeOption(pCxt, "daysPerFile", pOptions->pDaysPerFile, TSDB_MIN_DAYS_PER_FILE, TSDB_MAX_DAYS_PER_FILE); } if (TSDB_CODE_SUCCESS == code) { code = checkRangeOption(pCxt, "fsyncPeriod", pOptions->pFsyncPeriod, TSDB_MIN_FSYNC_PERIOD, TSDB_MAX_FSYNC_PERIOD); diff --git a/source/libs/parser/src/parUtil.c b/source/libs/parser/src/parUtil.c index 171324aa99..85d574d9e7 100644 --- a/source/libs/parser/src/parUtil.c +++ b/source/libs/parser/src/parUtil.c @@ -62,35 +62,39 @@ static char* getSyntaxErrFormat(int32_t errCode) { case TSDB_CODE_PAR_INTERVAL_VALUE_TOO_SMALL: return "This interval value is too small : %s"; case TSDB_CODE_PAR_DB_NOT_SPECIFIED: - return "db not specified"; + return "Database not specified"; case TSDB_CODE_PAR_INVALID_IDENTIFIER_NAME: return "Invalid identifier name : %s"; case TSDB_CODE_PAR_CORRESPONDING_STABLE_ERR: - return "corresponding super table not in this db"; + return "Corresponding super table not in this db"; case TSDB_CODE_PAR_INVALID_RANGE_OPTION: - return "invalid option %s: %"PRId64" valid range: [%d, %d]"; + return "Invalid option %s: %"PRId64" valid range: [%d, %d]"; case TSDB_CODE_PAR_INVALID_STR_OPTION: - return "invalid option %s: %s"; + return "Invalid option %s: %s"; case TSDB_CODE_PAR_INVALID_ENUM_OPTION: - return "invalid option %s: %"PRId64", only %d, %d allowed"; + return "Invalid option %s: %"PRId64", only %d, %d allowed"; case TSDB_CODE_PAR_INVALID_TTL_OPTION: - return "invalid option ttl: %"PRId64", should be greater than or equal to %d"; + return "Invalid option ttl: %"PRId64", should be greater than or equal to %d"; case TSDB_CODE_PAR_INVALID_KEEP_NUM: - return "invalid number of keep options"; + return "Invalid number of keep options"; case TSDB_CODE_PAR_INVALID_KEEP_ORDER: - return "invalid keep value, should be keep0 <= keep1 <= keep2"; + return "Invalid keep value, should be keep0 <= keep1 <= keep2"; case TSDB_CODE_PAR_INVALID_KEEP_VALUE: - return "invalid option keep: %d, %d, %d valid range: [%d, %d]"; + return "Invalid option keep: %d, %d, %d valid range: [%d, %d]"; case TSDB_CODE_PAR_INVALID_COMMENT_OPTION: - return "invalid option comment, length cannot exceed %d"; + return "Invalid option comment, length cannot exceed %d"; case TSDB_CODE_PAR_INVALID_F_RANGE_OPTION: - return "invalid option %s: %f valid range: [%d, %d]"; + return "Invalid option %s: %f valid range: [%d, %d]"; case TSDB_CODE_PAR_INVALID_ROLLUP_OPTION: - return "invalid option rollup: only one function is allowed"; + return "Invalid option rollup: only one function is allowed"; case TSDB_CODE_PAR_INVALID_RETENTIONS_OPTION: - return "invalid option retentions"; + return "Invalid option retentions"; case TSDB_CODE_PAR_GROUPBY_WINDOW_COEXIST: return "GROUP BY and WINDOW-clause can't be used together"; + case TSDB_CODE_PAR_INVALID_OPTION_UNIT: + return "Invalid option %s unit: %c, only m, h, d allowed"; + case TSDB_CODE_PAR_INVALID_KEEP_UNIT: + return "Invalid option keep unit: %c, %c, %c, only m, h, d allowed"; case TSDB_CODE_OUT_OF_MEMORY: return "Out of memory"; default: diff --git a/source/libs/parser/test/parserAstTest.cpp b/source/libs/parser/test/parserAstTest.cpp index d7e9b4a24d..149c317bd4 100644 --- a/source/libs/parser/test/parserAstTest.cpp +++ b/source/libs/parser/test/parserAstTest.cpp @@ -226,6 +226,16 @@ TEST_F(ParserTest, selectExpression) { ASSERT_TRUE(run()); } +TEST_F(ParserTest, selectCondition) { + setDatabase("root", "test"); + + bind("SELECT c1 FROM t1 where ts in (true, false)"); + ASSERT_TRUE(run()); + + bind("SELECT * FROM t1 where c1 > 10 and c1 is not null"); + ASSERT_TRUE(run()); +} + TEST_F(ParserTest, selectPseudoColumn) { setDatabase("root", "test");