You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@iotdb.apache.org by "YC27 (via GitHub)" <gi...@apache.org> on 2024/09/10 08:44:56 UTC
[PR] Support Load TsFile SQL: Load filePath with ('k'='v', ...) [iotdb]
YC27 opened a new pull request, #13466:
URL: https://github.com/apache/iotdb/pull/13466
(no comment)
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Load TsFile SQL: Load filePath with ('k'='v', ...) [iotdb]
Posted by "YC27 (via GitHub)" <gi...@apache.org>.
YC27 commented on code in PR #13466:
URL: https://github.com/apache/iotdb/pull/13466#discussion_r1751606019
##########
iotdb-core/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4:
##########
@@ -1141,6 +1141,22 @@ loadFileAttributeClause
| ONSUCCESS operator_eq (DELETE|NONE)
;
+// Load TsFile With
+loadFileWith
+ : LOAD fileName=STRING_LITERAL loadFileWithAttributeClauses
+ ;
+
+loadFileWithAttributeClauses
+ : WITH
+ LR_BRACKET
+ (loadFileWithAttributeClause COMMA)* loadFileWithAttributeClause?
+ RR_BRACKET
+ ;
+
+loadFileWithAttributeClause
Review Comment:
I think so, too
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Load: New SQL grammer -> `LOAD with ('k'='v', ...)` [iotdb]
Posted by "SteveYurongSu (via GitHub)" <gi...@apache.org>.
SteveYurongSu merged PR #13466:
URL: https://github.com/apache/iotdb/pull/13466
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Load: New SQL grammer -> `LOAD with ('k'='v', ...)` [iotdb]
Posted by "YC27 (via GitHub)" <gi...@apache.org>.
YC27 commented on code in PR #13466:
URL: https://github.com/apache/iotdb/pull/13466#discussion_r1766096878
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/parser/ASTVisitor.java:
##########
@@ -2005,6 +2006,28 @@ private long parseTimeValue(ConstantContext constant) {
@Override
public Statement visitLoadFile(IoTDBSqlParser.LoadFileContext ctx) {
try {
+ // if sql have with, return new load sql statement
+ if (ctx.loadFileWithAttributeClauses() != null) {
+ final Map<String, String> loadTsFileAttributes = new HashMap<>();
+ if (ctx.loadFileWithAttributeClauses() != null) {
Review Comment:
It is indeed one more time, there is no need to add. 👍
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Load TsFile SQL: Load filePath with ('k'='v', ...) [iotdb]
Posted by "Caideyipi (via GitHub)" <gi...@apache.org>.
Caideyipi commented on code in PR #13466:
URL: https://github.com/apache/iotdb/pull/13466#discussion_r1751562297
##########
iotdb-core/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4:
##########
@@ -1141,6 +1141,22 @@ loadFileAttributeClause
| ONSUCCESS operator_eq (DELETE|NONE)
;
+// Load TsFile With
+loadFileWith
Review Comment:
Maybe set the upper one to "loadFileWithAttribute"?
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/parser/ASTVisitor.java:
##########
@@ -2046,6 +2047,28 @@ private void parseLoadFileAttributeClause(
}
}
+ @Override
+ public Statement visitLoadFileWith(IoTDBSqlParser.LoadFileWithContext ctx) {
+ try {
+ final Map<String, String> loadTsFileAttributes = new HashMap<>();
+ if (ctx.loadFileWithAttributeClauses() != null) {
+ for (IoTDBSqlParser.LoadFileWithAttributeClauseContext attributeContext :
+ ctx.loadFileWithAttributeClauses().loadFileWithAttributeClause()) {
+ final String key = parseStringLiteral(attributeContext.loadFileWithKey.getText());
+ final String value = parseStringLiteral(attributeContext.loadFileWithValue.getText());
+
+ LoadTsFileConstant.validateLoadTsFileParam(key, value);
Review Comment:
Are the key and value case-sensitive?
##########
iotdb-core/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4:
##########
@@ -1141,6 +1141,22 @@ loadFileAttributeClause
| ONSUCCESS operator_eq (DELETE|NONE)
;
+// Load TsFile With
+loadFileWith
+ : LOAD fileName=STRING_LITERAL loadFileWithAttributeClauses
+ ;
+
+loadFileWithAttributeClauses
Review Comment:
Better name it loadFileAttributes?
##########
iotdb-core/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4:
##########
@@ -1141,6 +1141,22 @@ loadFileAttributeClause
| ONSUCCESS operator_eq (DELETE|NONE)
;
+// Load TsFile With
+loadFileWith
+ : LOAD fileName=STRING_LITERAL loadFileWithAttributeClauses
+ ;
+
+loadFileWithAttributeClauses
+ : WITH
+ LR_BRACKET
+ (loadFileWithAttributeClause COMMA)* loadFileWithAttributeClause?
+ RR_BRACKET
+ ;
+
+loadFileWithAttributeClause
Review Comment:
I think those attributes in pipes can be unified.. Because there are so many "string = string"s in antlr
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/crud/LoadTsFileConstant.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.statement.crud;
+
+import org.apache.iotdb.db.conf.IoTDBDescriptor;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+public class LoadTsFileConstant {
Review Comment:
May add a private constructor
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/statement/crud/LoadTsFileConstant.java:
##########
@@ -0,0 +1,92 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.iotdb.db.queryengine.plan.statement.crud;
+
+import org.apache.iotdb.db.conf.IoTDBDescriptor;
+
+import org.apache.commons.lang3.StringUtils;
+
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+public class LoadTsFileConstant {
+
+ // constant Statement
+ public static final String DATABASE_LEVEL_KEY = "database-level";
Review Comment:
These constants can be private
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Load TsFile SQL: Load filePath with ('k'='v', ...) [iotdb]
Posted by "YC27 (via GitHub)" <gi...@apache.org>.
YC27 commented on code in PR #13466:
URL: https://github.com/apache/iotdb/pull/13466#discussion_r1751606019
##########
iotdb-core/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4:
##########
@@ -1141,6 +1141,22 @@ loadFileAttributeClause
| ONSUCCESS operator_eq (DELETE|NONE)
;
+// Load TsFile With
+loadFileWith
+ : LOAD fileName=STRING_LITERAL loadFileWithAttributeClauses
+ ;
+
+loadFileWithAttributeClauses
+ : WITH
+ LR_BRACKET
+ (loadFileWithAttributeClause COMMA)* loadFileWithAttributeClause?
+ RR_BRACKET
+ ;
+
+loadFileWithAttributeClause
Review Comment:
I think so, too
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Load: New SQL grammer -> `LOAD with ('k'='v', ...)` [iotdb]
Posted by "MiniSho (via GitHub)" <gi...@apache.org>.
MiniSho commented on code in PR #13466:
URL: https://github.com/apache/iotdb/pull/13466#discussion_r1766088587
##########
iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/parser/ASTVisitor.java:
##########
@@ -2005,6 +2006,28 @@ private long parseTimeValue(ConstantContext constant) {
@Override
public Statement visitLoadFile(IoTDBSqlParser.LoadFileContext ctx) {
try {
+ // if sql have with, return new load sql statement
+ if (ctx.loadFileWithAttributeClauses() != null) {
+ final Map<String, String> loadTsFileAttributes = new HashMap<>();
+ if (ctx.loadFileWithAttributeClauses() != null) {
Review Comment:
I'm wondering if there's a specific reason for this nested check, or if the inner `if` statement might be unnecessary?
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Support Load TsFile SQL: Load filePath with ('k'='v', ...) [iotdb]
Posted by "YC27 (via GitHub)" <gi...@apache.org>.
YC27 commented on code in PR #13466:
URL: https://github.com/apache/iotdb/pull/13466#discussion_r1751605557
##########
iotdb-core/antlr/src/main/antlr4/org/apache/iotdb/db/qp/sql/IoTDBSqlParser.g4:
##########
@@ -1141,6 +1141,22 @@ loadFileAttributeClause
| ONSUCCESS operator_eq (DELETE|NONE)
;
+// Load TsFile With
+loadFileWith
+ : LOAD fileName=STRING_LITERAL loadFileWithAttributeClauses
+ ;
+
+loadFileWithAttributeClauses
+ : WITH
+ LR_BRACKET
+ (loadFileWithAttributeClause COMMA)* loadFileWithAttributeClause?
+ RR_BRACKET
+ ;
+
+loadFileWithAttributeClause
Review Comment:
I think so, too
--
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.
To unsubscribe, e-mail: reviews-unsubscribe@iotdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org