You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@seatunnel.apache.org by GitBox <gi...@apache.org> on 2022/01/03 03:56:55 UTC
[GitHub] [incubator-seatunnel] xbkaishui opened a new pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
xbkaishui opened a new pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930
<!--
Thank you for contributing to SeaTunnel! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
## Contribution Checklist
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-seatunnel/issues).
- Name the pull request in the form "[SeaTunnel #XXXX] [component] Title of the pull request", where *SeaTunnel #XXXX* should be replaced by the actual issue number.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
Add SqlStatementSplitter test case
Fix sql spliter single line bug
## Check list
* [ ] Code changed are covered with tests, or it does not need tests for reason:
* [ ] Change does not need document change, or I will submit document change to https://github.com/apache/incubator-seatunnel-website later
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780728943
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ // this is bad case, multi sql should split by line \n
+ String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertEquals(sqls.size(), 1);
Review comment:
We should put constants in first param. `assertEquals(1, sqls.size());`
```
/**
* Asserts that two objects are equal. If they are not, an
* {@link AssertionError} without a message is thrown. If
* <code>expected</code> and <code>actual</code> are <code>null</code>,
* they are considered equal.
*
* @param expected expected value
* @param actual the value to check against <code>expected</code>
*/
public static void assertEquals(Object expected, Object actual) {
assertEquals(null, expected, actual);
}
```
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778579361
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
when `--` is the prefix of the statement, the whole statement is comment, we shouldn't split it, please revert this.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1007220680
Thanks your contribution! Overall LGTM, left one place need to do that let's revert the split logic in this patch.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780090186
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, the sql statements like following, it will return wrong statements after apply this patch.
```
String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
```
And, if `--` is the prefix of the statement, we should not handle it.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 edited a comment on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 edited a comment on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1010778306
hi @CalvinKirs, workflows are waiting to be approved.
hi @xbkaishui, can you check the following link, is there an enable button?
https://github.com/xbkaishui/incubator-seatunnel/actions
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780863498
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -70,11 +73,11 @@
*/
private static String normalizeLine(List<String> buffer) {
return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
Review comment:
nit: sorry, seem here still didn't been updated.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780591323
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
got it, reverted
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778580012
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
Review comment:
Let's use `junit` dependency.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780065718
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ String sqlContent = "--test is a comment \n select * from dual; select now();";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertThat(sqls.size(), is(2));
Review comment:
done, please check
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
Review comment:
done, please check
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780749897
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,45 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ // this is bad case, multi sql should split by line \n
+ String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertEquals(sqls.size(), 1);
Review comment:
done, thanks
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778550817
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
please see test case, normalizeStatementsWithMultiSqls, if mult sql in one lines, it will wrong to return one string
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778583331
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, we already handle the logic in `SqlStatementSplitter#splitContent`, it will return the statements.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1010778306
hi @CalvinKirs, workflows are waiting to be approved.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778550902
##########
File path: seatunnel-core/seatunnel-core-sql/pom.xml
##########
@@ -66,5 +66,10 @@
<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
<version>${flink.version}</version>
</dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
Review comment:
ok
> Hi, can you format your code, there is a codestyle file in our tools directory, you can import it to check, or use `mvn checkstyle:check` to check
got it will change later
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780728981
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
flink sql parser can't split mluti statements by default.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780090332
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
Review comment:
Good job.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780090186
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, if sql statements like following, it will return wrong statements.
```
String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
```
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778579870
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ String sqlContent = "--test is a comment \n select * from dual; select now();";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertThat(sqls.size(), is(2));
Review comment:
please convert to `Assert.assertEquals`.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r777875307
##########
File path: seatunnel-core/seatunnel-core-sql/pom.xml
##########
@@ -66,5 +66,10 @@
<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
<version>${flink.version}</version>
</dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
Review comment:
we can remove `<scope>test</scope>`
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780090844
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ String sqlContent = "--test is a comment \n select * from dual; select now();";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertThat(sqls.size(), is(2));
Review comment:
Good.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780090186
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, if sql statements like following, it will return wrong statements.
```
String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
```
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
Review comment:
Good job.
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ String sqlContent = "--test is a comment \n select * from dual; select now();";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertThat(sqls.size(), is(2));
Review comment:
Good.
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, if sql statements like following, it will return wrong statements.
```
String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
```
And, if `--` is the prefix of the statement, we should not handle it.
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, the sql statements like following, it will return wrong statements after apply this patch.
```
String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
```
And, if `--` is the prefix of the statement, we should not handle it.
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
We can not split sql statements by `;` directly.
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
We should not split sql statements by `;` directly.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1010789896
> hi @CalvinKirs, workflows are waiting to be approved.
>
> hi @xbkaishui, can you check the following link, is there an enable button? https://github.com/xbkaishui/incubator-seatunnel/actions
Thanks for the reminder~
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778579361
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
when `--` is the prefix of the statement, the whole statement is comment, we shouldn't split it, please revert this.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780096522
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
We can not split sql statements by `;` directly.
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
We should not split sql statements by `;` directly.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780591608
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
ok, is it better to use flink sql parser to split sqls ? current logic is not concise
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780064324
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
sorry, don't got it
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780090186
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, if sql statements like following, it will return wrong statements.
```
String sqlContent = "--test is a comment \n select * from dual; select now(); select * from logs where log_content like ';'";
```
And, if `--` is the prefix of the statement, we should not handle it.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] CalvinKirs merged pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
CalvinKirs merged pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780064324
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
sorry, don't got it
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.MatcherAssert.assertThat;
+
+public class SqlStatementSplitterTest {
+
+ @Test
+ public void normalizeStatementsWithMultiSqls() {
+ String sqlContent = "--test is a comment \n select * from dual; select now();";
+ List<String> sqls = SqlStatementSplitter.normalizeStatements(sqlContent);
+ assertThat(sqls.size(), is(2));
Review comment:
done, please check
##########
File path: seatunnel-core/seatunnel-core-sql/src/test/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitterTest.java
##########
@@ -0,0 +1,46 @@
+/*
+ * 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.seatunnel.core.sql.splitter;
+
+import java.util.List;
+import org.junit.Test;
+
+import static org.hamcrest.CoreMatchers.is;
Review comment:
done, please check
##########
File path: seatunnel-core/seatunnel-core-sql/pom.xml
##########
@@ -66,5 +66,10 @@
<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
<version>${flink.version}</version>
</dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
Review comment:
done, please check
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
got it, reverted
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
+ String sqls = buffer.stream()
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, EMPTY_STR))
+ .collect(Collectors.joining(LINE_SEPARATOR));
+ return Arrays.asList(sqls.split(SEMICOLON));
Review comment:
ok, is it better to use flink sql parser to split sqls ? current logic is not concise
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] CalvinKirs edited a comment on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
CalvinKirs edited a comment on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1004548904
Hi, can you format your code, there is a codestyle file in our tools directory, you can import it to check, or use `mvn checkstyle:check` to check
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r780065799
##########
File path: seatunnel-core/seatunnel-core-sql/pom.xml
##########
@@ -66,5 +66,10 @@
<artifactId>flink-streaming-scala_${scala.binary.version}</artifactId>
<version>${flink.version}</version>
</dependency>
+ <dependency>
+ <groupId>junit</groupId>
+ <artifactId>junit</artifactId>
+ <scope>test</scope>
Review comment:
done, please check
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r777876584
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, have a question why change `String` to `List<String>`?
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, have a question that why change `String` to `List<String>`?
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] CalvinKirs commented on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
CalvinKirs commented on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1004548904
Hi, can you format your code, there is a codestyle file in our tools directory, you can import it to check, or use mvn `checkstyle:check` to check
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778583331
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, we already handle the logic in `SqlStatementSplitter#splitContent`, it will return the statements, you can run your test case directly.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r778583331
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -50,31 +54,32 @@
List<String> statements = new ArrayList<>();
List<String> buffer = new ArrayList<>();
- for (String line : content.split("\n")) {
+ for (String line : content.split(LINE_SEPARATOR)) {
if (isEndOfStatement(line)) {
buffer.add(line);
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
buffer.clear();
} else {
buffer.add(line);
}
}
if (!buffer.isEmpty()) {
- statements.add(normalizeLine(buffer));
+ statements.addAll(normalizeLine(buffer));
}
return statements;
}
/**
* Remove comment lines.
*/
- private static String normalizeLine(List<String> buffer) {
- return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ private static List<String> normalizeLine(List<String> buffer) {
Review comment:
hi, we already handle the logic in `SqlStatementSplitter#splitContent`, it will return the statements, you can try your test case directly.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] leo65535 commented on pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
leo65535 commented on pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#issuecomment-1007220680
Thanks your contribution! Overall LGTM, left one place need to do that let's revert the split logic in this patch.
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-seatunnel] xbkaishui commented on a change in pull request #930: [SeaTunnel #783][UT] Add SqlStatementSplitter test case
Posted by GitBox <gi...@apache.org>.
xbkaishui commented on a change in pull request #930:
URL: https://github.com/apache/incubator-seatunnel/pull/930#discussion_r782789346
##########
File path: seatunnel-core/seatunnel-core-sql/src/main/java/org/apache/seatunnel/core/sql/splitter/SqlStatementSplitter.java
##########
@@ -70,11 +73,11 @@
*/
private static String normalizeLine(List<String> buffer) {
return buffer.stream()
- .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
- .collect(Collectors.joining("\n"));
+ .map(statementLine -> statementLine.replaceAll(BEGINNING_COMMENT_MASK, ""))
Review comment:
done
--
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: commits-unsubscribe@seatunnel.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org