You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by GitBox <gi...@apache.org> on 2021/05/21 03:09:08 UTC
[GitHub] [iotdb] liutaohua commented on a change in pull request #3236: [IOTDB-1022] ANTLR Grammar & Logical Operator for Arithmetic Operations and Nested Operations in SELECT Clauses
liutaohua commented on a change in pull request #3236:
URL: https://github.com/apache/iotdb/pull/3236#discussion_r636603307
##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/SqlBase.g4
##########
@@ -103,71 +103,30 @@ statement
| START TRIGGER triggerName=ID #startTrigger
| STOP TRIGGER triggerName=ID #stopTrigger
| SHOW TRIGGERS #showTriggers
- | SELECT topClause? selectElements
- fromClause
- whereClause?
- specialClause? #selectStatement
+ | selectClause fromClause whereClause? specialClause? #selectStatement
;
-selectElements
- : aggregationCall (COMMA aggregationCall)* #aggregationElement
- | tableCall (COMMA tableCall)* #tableElement
- | lastClause #lastElement
- | asClause (COMMA asClause)* #asElement
- | functionAsClause (COMMA functionAsClause)* #functionAsElement
- ;
-
-aggregationCall
- : builtInFunctionCall
- | udfCall
- ;
-
-tableCall
- : suffixPath
- | udfCall
- | SINGLE_QUOTE_STRING_LITERAL
- ;
-
-udfCall
- : udfName=ID LR_BRACKET udfSuffixPaths udfAttribute* RR_BRACKET
- ;
-
-udfSuffixPaths
- : suffixPath (COMMA suffixPath)*
- ;
-
-udfAttribute
- : COMMA udfAttributeKey=stringLiteral OPERATOR_EQ udfAttributeValue=stringLiteral
- ;
-
-builtInFunctionCall
- : functionName LR_BRACKET suffixPath RR_BRACKET
- ;
-
-functionName
- : MIN_TIME
- | MAX_TIME
- | MIN_VALUE
- | MAX_VALUE
- | COUNT
- | AVG
- | FIRST_VALUE
- | SUM
- | LAST_VALUE
- ;
+selectClause
+ : SELECT (LAST | topClause)? resultColumn (COMMA resultColumn)*
+ ;
-functionAsClause
- : builtInFunctionCall (AS ID)?
- ;
+resultColumn
+ : expression (AS ID)?
+ ;
-lastClause
- : LAST suffixPath (COMMA suffixPath)*
- | LAST asClause (COMMA asClause)*
- ;
+expression
+ : LR_BRACKET unary=expression RR_BRACKET
+ | (PLUS | MINUS) unary=expression
+ | leftExpression=expression (STAR | DIV | MOD) rightExpression=expression
+ | leftExpression=expression (PLUS | MINUS) rightExpression=expression
+ | functionName=suffixPath LR_BRACKET expression (COMMA expression)* functionAttribute* RR_BRACKET
+ | suffixPath
Review comment:
`SuffixPath` is so open that it supports almost everything. We should restrict it to `+ - * / % mod `and other special characters
##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/SqlBase.g4
##########
@@ -103,71 +103,30 @@ statement
| START TRIGGER triggerName=ID #startTrigger
| STOP TRIGGER triggerName=ID #stopTrigger
| SHOW TRIGGERS #showTriggers
- | SELECT topClause? selectElements
- fromClause
- whereClause?
- specialClause? #selectStatement
+ | selectClause fromClause whereClause? specialClause? #selectStatement
;
-selectElements
- : aggregationCall (COMMA aggregationCall)* #aggregationElement
- | tableCall (COMMA tableCall)* #tableElement
- | lastClause #lastElement
- | asClause (COMMA asClause)* #asElement
- | functionAsClause (COMMA functionAsClause)* #functionAsElement
- ;
-
-aggregationCall
- : builtInFunctionCall
- | udfCall
- ;
-
-tableCall
- : suffixPath
- | udfCall
- | SINGLE_QUOTE_STRING_LITERAL
- ;
-
-udfCall
- : udfName=ID LR_BRACKET udfSuffixPaths udfAttribute* RR_BRACKET
- ;
-
-udfSuffixPaths
- : suffixPath (COMMA suffixPath)*
- ;
-
-udfAttribute
- : COMMA udfAttributeKey=stringLiteral OPERATOR_EQ udfAttributeValue=stringLiteral
- ;
-
-builtInFunctionCall
- : functionName LR_BRACKET suffixPath RR_BRACKET
- ;
-
-functionName
- : MIN_TIME
- | MAX_TIME
- | MIN_VALUE
- | MAX_VALUE
- | COUNT
- | AVG
- | FIRST_VALUE
- | SUM
- | LAST_VALUE
- ;
+selectClause
+ : SELECT (LAST | topClause)? resultColumn (COMMA resultColumn)*
+ ;
-functionAsClause
- : builtInFunctionCall (AS ID)?
- ;
+resultColumn
+ : expression (AS ID)?
+ ;
-lastClause
- : LAST suffixPath (COMMA suffixPath)*
- | LAST asClause (COMMA asClause)*
- ;
+expression
+ : LR_BRACKET unary=expression RR_BRACKET
+ | (PLUS | MINUS) unary=expression
+ | leftExpression=expression (STAR | DIV | MOD) rightExpression=expression
+ | leftExpression=expression (PLUS | MINUS) rightExpression=expression
+ | functionName=suffixPath LR_BRACKET expression (COMMA expression)* functionAttribute* RR_BRACKET
+ | suffixPath
+ | literal=SINGLE_QUOTE_STRING_LITERAL
Review comment:
Special branches that are only used as `align by device` are not recommended to be retained in recursion.
I think it will affect the speed of parsing.
##########
File path: antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/SqlBase.g4
##########
@@ -1370,6 +1281,10 @@ MINUS : '-';
PLUS : '+';
+DIV : '/';
+
+MOD : M O D | '%';
Review comment:
`MOD` keywords are not recommended, which makes it necessary to reserve more keywords
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org