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/04/27 23:36:16 UTC
[GitHub] [incubator-seatunnel] wanghuan2054 opened a new pull request, #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
wanghuan2054 opened a new pull request, #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761
<!--
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 "[Feature] [component] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Minor fixes should be named following this pattern: `[hotfix] [docs] Fix typo in README.md doc`.
-->
## Purpose of this pull request
<!-- Describe the purpose of this pull request. For example: This pull request adds checkstyle plugin.-->
## Check list
* [x] Code changed are covered with tests, or it does not need tests for reason:
* [x] If any new Jar binary package adding in your PR, please add License Notice according
[New License Guide](https://github.com/apache/incubator-seatunnel/blob/dev/docs/en/contribution/new-license.md)
* [x] If necessary, please update the documentation to describe the new feature. https://github.com/apache/incubator-seatunnel/tree/dev/docs
--
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] ruanwenjun commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1119238222
> > FlinkSQL already has built-in replace function, I suggest that we don't need to add this replace transform. What do you think? @CalvinKirs @BenJFan @legendtkl
>
> I have some confuse with transform, why we need bind with SQL？Just use one source and transform then generate another source maybe better？
If our transform function is powerful, then we don't need to bind with SQL. But the fact is that the Flink-SQL, Spark-SQL is more powerful, and easy to use. My concern is that we developed a lot of transform plugin but no-one use our plugin, they directly use Flink-SQL, this will cost our attention to maintain.
--
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] wanghuan2054 commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1111676079
邮件已收到 ，谢谢 ！
--
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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r861943522
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,80 @@
+package org.apache.seatunnel.flink.transform;
+
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.streaming.api.datastream.DataStream;
+import org.apache.flink.types.Row;
+import org.apache.seatunnel.common.config.CheckConfigUtil;
+import org.apache.seatunnel.common.config.CheckResult;
+import org.apache.seatunnel.flink.FlinkEnvironment;
+import org.apache.seatunnel.flink.batch.FlinkBatchTransform;
+import org.apache.seatunnel.flink.stream.FlinkStreamTransform;
+import org.apache.seatunnel.shade.com.typesafe.config.Config;
+
+public class Replace implements FlinkStreamTransform, FlinkBatchTransform {
+
+ private Config config;
+
+ private static final String PATTERN = "pattern";
+ private static final String REPLACEMENT = "replacement";
+ private static final String ISREGEX = "is_regex";
+ private static final String REPLACEFIRST = "replace_first";
+
+ private String pattern = "";
+ private String replacement = "";
+ private Boolean isRegex = false;
+ private Boolean replaceFirst = false;
+
+
+ @Override
+ public void setConfig(Config config) {
+ this.config = config;
+ }
+
+ @Override
+ public Config getConfig() {
+ return config;
+ }
+
+ @Override
+ public CheckResult checkConfig() {
+ return CheckConfigUtil.checkAllExists(config);
Review Comment:
ok, I have modified
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/ScalarReplace.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.seatunnel.flink.transform;
Review Comment:
ok, I have modified
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/test/java/org/apache/seatunnel/flink/transform/TestReplace.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.seatunnel.flink.transform;
Review Comment:
ok, I have modified
--
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] BenJFan commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
BenJFan commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1111777298
@ruanwenjun Hi wenjun, please make workflow start. 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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r860430952
##########
docs/en/transform/replace.md:
##########
@@ -67,13 +67,103 @@ Use `Replace` as udf in sql.
Replace {
fields = "_replaced"
pattern = "([^ ]*) ([^ ]*)"
- replacement = "$2
+ replacement = "$2“
isRegex = true
replaceFirst = true
}
- # Use the split function (confirm that the fake table exists)
+ # Use the replace function (confirm that the fake table exists)
sql {
sql = "select * from (select raw_message, replace(raw_message) as info_row from fake) t1"
}
```
+
+## Options(Flink)
+
+| name | type | required | default value |
+| ----- | ------ | -------- |---------------|
+| pattern | string | no | - |
+| replacement | string | no | - |
+| is_regex | boolean | no | false |
+| replace_first | boolean | no | false |
+| common-options | string | no | - |
+
+### pattern [string]
+
+the regular expression to which this string is to be matched or a common string literal, the default is ""
Review Comment:
ok ， I have modified
##########
docs/en/transform/replace.md:
##########
@@ -67,13 +67,103 @@ Use `Replace` as udf in sql.
Replace {
fields = "_replaced"
pattern = "([^ ]*) ([^ ]*)"
- replacement = "$2
+ replacement = "$2“
isRegex = true
replaceFirst = true
}
- # Use the split function (confirm that the fake table exists)
+ # Use the replace function (confirm that the fake table exists)
sql {
sql = "select * from (select raw_message, replace(raw_message) as info_row from fake) t1"
}
```
+
+## Options(Flink)
+
+| name | type | required | default value |
+| ----- | ------ | -------- |---------------|
+| pattern | string | no | - |
+| replacement | string | no | - |
+| is_regex | boolean | no | false |
+| replace_first | boolean | no | false |
+| common-options | string | no | - |
+
+### pattern [string]
+
+the regular expression to which this string is to be matched or a common string literal, the default is ""
+
+### replacement [string]
+
+the string to be substituted for each match, the default is ""
+
+### is_regex [boolean]
+
+Whether or not to interpret the pattern as a regex (true) or string literal (false), the dafault is false
+
+### replace_first [boolean]
+
+when replace_first set true , The string constructed by replacing the first matching subsequence by the replacement
+string,
Review Comment:
ok ， I have modified
--
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] ruanwenjun closed pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
ruanwenjun closed pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
URL: https://github.com/apache/incubator-seatunnel/pull/1761
--
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] BenJFan commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
BenJFan commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r860402414
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,105 @@
+package org.apache.seatunnel.flink.transform;
+
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.streaming.api.datastream.DataStream;
+import org.apache.flink.types.Row;
+import org.apache.seatunnel.common.config.CheckConfigUtil;
+import org.apache.seatunnel.common.config.CheckResult;
+import org.apache.seatunnel.flink.FlinkEnvironment;
+import org.apache.seatunnel.flink.batch.FlinkBatchTransform;
+import org.apache.seatunnel.flink.stream.FlinkStreamTransform;
+import org.apache.seatunnel.shade.com.typesafe.config.Config;
+
+public class Replace implements FlinkStreamTransform, FlinkBatchTransform {
+
+ private Config config;
+
+ private static final String PATTERN = "pattern";
+ private static final String REPLACEMENT = "replacement";
+ private static final String ISREGEX = "is_regex";
+ private static final String REPLACEFIRST = "replace_first";
+
+ private String pattern = "";
+ private String replacement = "";
+ private Boolean isRegex = false;
+ private Boolean replaceFirst = false;
+
+
+ /**
+ * @param config
+ */
+ @Override
+ public void setConfig(Config config) {
+ this.config = config;
+ }
+
+ /**
+ * @return
Review Comment:
Useless comment. Just remove it will be better.
##########
docs/en/transform/replace.md:
##########
@@ -67,13 +67,103 @@ Use `Replace` as udf in sql.
Replace {
fields = "_replaced"
pattern = "([^ ]*) ([^ ]*)"
- replacement = "$2
+ replacement = "$2“
isRegex = true
replaceFirst = true
}
- # Use the split function (confirm that the fake table exists)
+ # Use the replace function (confirm that the fake table exists)
sql {
sql = "select * from (select raw_message, replace(raw_message) as info_row from fake) t1"
}
```
+
+## Options(Flink)
+
+| name | type | required | default value |
+| ----- | ------ | -------- |---------------|
+| pattern | string | no | - |
+| replacement | string | no | - |
+| is_regex | boolean | no | false |
+| replace_first | boolean | no | false |
+| common-options | string | no | - |
+
+### pattern [string]
+
+the regular expression to which this string is to be matched or a common string literal, the default is ""
+
+### replacement [string]
+
+the string to be substituted for each match, the default is ""
+
+### is_regex [boolean]
+
+Whether or not to interpret the pattern as a regex (true) or string literal (false), the dafault is false
+
+### replace_first [boolean]
+
+when replace_first set true , The string constructed by replacing the first matching subsequence by the replacement
+string,
Review Comment:
Wrong line break
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,105 @@
+package org.apache.seatunnel.flink.transform;
+
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.streaming.api.datastream.DataStream;
+import org.apache.flink.types.Row;
+import org.apache.seatunnel.common.config.CheckConfigUtil;
+import org.apache.seatunnel.common.config.CheckResult;
+import org.apache.seatunnel.flink.FlinkEnvironment;
+import org.apache.seatunnel.flink.batch.FlinkBatchTransform;
+import org.apache.seatunnel.flink.stream.FlinkStreamTransform;
+import org.apache.seatunnel.shade.com.typesafe.config.Config;
+
+public class Replace implements FlinkStreamTransform, FlinkBatchTransform {
+
+ private Config config;
+
+ private static final String PATTERN = "pattern";
+ private static final String REPLACEMENT = "replacement";
+ private static final String ISREGEX = "is_regex";
+ private static final String REPLACEFIRST = "replace_first";
+
+ private String pattern = "";
+ private String replacement = "";
+ private Boolean isRegex = false;
+ private Boolean replaceFirst = false;
+
+
+ /**
+ * @param config
+ */
+ @Override
+ public void setConfig(Config config) {
+ this.config = config;
+ }
+
+ /**
+ * @return
Review Comment:
Useless comment. Just remove it will be better.
##########
docs/en/transform/replace.md:
##########
@@ -67,13 +67,103 @@ Use `Replace` as udf in sql.
Replace {
fields = "_replaced"
pattern = "([^ ]*) ([^ ]*)"
- replacement = "$2
+ replacement = "$2“
isRegex = true
replaceFirst = true
}
- # Use the split function (confirm that the fake table exists)
+ # Use the replace function (confirm that the fake table exists)
sql {
sql = "select * from (select raw_message, replace(raw_message) as info_row from fake) t1"
}
```
+
+## Options(Flink)
+
+| name | type | required | default value |
+| ----- | ------ | -------- |---------------|
+| pattern | string | no | - |
+| replacement | string | no | - |
+| is_regex | boolean | no | false |
+| replace_first | boolean | no | false |
+| common-options | string | no | - |
+
+### pattern [string]
+
+the regular expression to which this string is to be matched or a common string literal, the default is ""
Review Comment:
Use uppercase in start.
--
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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r860431060
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,105 @@
+package org.apache.seatunnel.flink.transform;
+
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.streaming.api.datastream.DataStream;
+import org.apache.flink.types.Row;
+import org.apache.seatunnel.common.config.CheckConfigUtil;
+import org.apache.seatunnel.common.config.CheckResult;
+import org.apache.seatunnel.flink.FlinkEnvironment;
+import org.apache.seatunnel.flink.batch.FlinkBatchTransform;
+import org.apache.seatunnel.flink.stream.FlinkStreamTransform;
+import org.apache.seatunnel.shade.com.typesafe.config.Config;
+
+public class Replace implements FlinkStreamTransform, FlinkBatchTransform {
+
+ private Config config;
+
+ private static final String PATTERN = "pattern";
+ private static final String REPLACEMENT = "replacement";
+ private static final String ISREGEX = "is_regex";
+ private static final String REPLACEFIRST = "replace_first";
+
+ private String pattern = "";
+ private String replacement = "";
+ private Boolean isRegex = false;
+ private Boolean replaceFirst = false;
+
+
+ /**
+ * @param config
+ */
+ @Override
+ public void setConfig(Config config) {
+ this.config = config;
+ }
+
+ /**
+ * @return
Review Comment:
ok ， I have modified
--
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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r861945173
##########
docs/en/transform/replace.md:
##########
@@ -1,16 +1,17 @@
-# Json
+# Replace
## Description
-Examines string value in a given field and replaces substring of the string value that matches the given string literal or regexes with the given replacement.
+Examines string value in a given field and replaces substring of the string value that matches the given string literal
+or regexes with the given replacement.
:::tip
-This transform **ONLY** supported by Spark.
+This transform can supported by Spark and Flink,There are slight differences between the two engine configuration items.
:::
-## Options
+## Options(Spark)
Review Comment:
ok, I have modified
--
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] ruanwenjun commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r860516993
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/pom.xml:
##########
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>seatunnel-transforms-flink</artifactId>
+ <groupId>org.apache.seatunnel</groupId>
+ <version>2.1.1-SNAPSHOT</version>
Review Comment:
```suggestion
<version>${revision}</version>
```
##########
docs/zh-CN/flink/configuration/transform-plugins/Replace.md:
##########
@@ -0,0 +1,94 @@
+# Replace
Review Comment:
There is no need to provide Chinese documentation, We will find a tool to help generate the Chinese documentation, we have already discussed in this [mail thread ](https://lists.apache.org/thread/j1vo1yccqp79bv69bzokbqq3n58zytql)
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,80 @@
+package org.apache.seatunnel.flink.transform;
Review Comment:
Missing Apache license header.
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/pom.xml:
##########
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>seatunnel-transforms-flink</artifactId>
+ <groupId>org.apache.seatunnel</groupId>
+ <version>2.1.1-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>seatunnel-transform-flink-replace</artifactId>
+
+ <properties>
+ <junit.version>4.4</junit.version>
Review Comment:
We have defined the JUnit version in root `pom.xml`, so you don't need to define a version here, this will make it hard to manage the version.
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/ScalarReplace.java:
##########
@@ -0,0 +1,35 @@
+package org.apache.seatunnel.flink.transform;
Review Comment:
Missing Apache license header.
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,80 @@
+package org.apache.seatunnel.flink.transform;
+
+import org.apache.flink.api.java.DataSet;
+import org.apache.flink.streaming.api.datastream.DataStream;
+import org.apache.flink.types.Row;
+import org.apache.seatunnel.common.config.CheckConfigUtil;
+import org.apache.seatunnel.common.config.CheckResult;
+import org.apache.seatunnel.flink.FlinkEnvironment;
+import org.apache.seatunnel.flink.batch.FlinkBatchTransform;
+import org.apache.seatunnel.flink.stream.FlinkStreamTransform;
+import org.apache.seatunnel.shade.com.typesafe.config.Config;
+
+public class Replace implements FlinkStreamTransform, FlinkBatchTransform {
+
+ private Config config;
+
+ private static final String PATTERN = "pattern";
+ private static final String REPLACEMENT = "replacement";
+ private static final String ISREGEX = "is_regex";
+ private static final String REPLACEFIRST = "replace_first";
+
+ private String pattern = "";
+ private String replacement = "";
+ private Boolean isRegex = false;
+ private Boolean replaceFirst = false;
+
+
+ @Override
+ public void setConfig(Config config) {
+ this.config = config;
+ }
+
+ @Override
+ public Config getConfig() {
+ return config;
+ }
+
+ @Override
+ public CheckResult checkConfig() {
+ return CheckConfigUtil.checkAllExists(config);
Review Comment:
You may need to add which config key you want to check.
##########
docs/en/transform/replace.md:
##########
@@ -1,16 +1,17 @@
-# Json
+# Replace
## Description
-Examines string value in a given field and replaces substring of the string value that matches the given string literal or regexes with the given replacement.
+Examines string value in a given field and replaces substring of the string value that matches the given string literal
+or regexes with the given replacement.
:::tip
-This transform **ONLY** supported by Spark.
+This transform can supported by Spark and Flink,There are slight differences between the two engine configuration items.
:::
-## Options
+## Options(Spark)
Review Comment:
It's better to use `<Tabs>` to manage the parameters in different engine. And it would be better to implement this plugin with same parameters in different engine.
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/test/java/org/apache/seatunnel/flink/transform/TestReplace.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.seatunnel.flink.transform;
Review Comment:
Missing Apache license header.
--
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] BenJFan commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
BenJFan commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1119217033
> FlinkSQL already has built-in replace function, I suggest that we don't need to add this replace transform. What do you think? @CalvinKirs @BenJFan @legendtkl
I have some confuse with transform, why we need bind with SQL？Just use one source and transform then generate another source maybe better？
--
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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r861942729
##########
docs/zh-CN/flink/configuration/transform-plugins/Replace.md:
##########
@@ -0,0 +1,94 @@
+# Replace
Review Comment:
ok, I remove 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] legendtkl commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
legendtkl commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1119213161
Yes, the built-in function `REPLACE` and `REGEXP_REPLACE` are powerful and seem to be able to cover the usage of this PR. cc @ruanwenjun
--
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] ruanwenjun commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1119680620
@wanghuan2054 Thanks for your great working, after discuss in this PR, we think this transform plugin is not needed. I will close this PR, if you have any idea, welcome to discuss in dev@seatunnel.apache.org.
--
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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r860508131
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/test/java/org/apache/seatunnel/flink/transform/TestReplace.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.seatunnel.flink.transform;
+
+
+import junit.framework.Assert;
+import org.junit.Test;
+
+public class TestReplace {
+
+ /**
+ * Replace the string that the first regular expression matches
+ * Input : Tom's phone number is 123456789 , he's age is 24
+ * Output : Tom's phone number is * , he's age is 24
+ */
+ @Test
+ public void testExample01() {
+ String input = "Tom's phone number is 123456789 , he's age is 24";
+ String output = "Tom's phone number is * , he's age is 24";
+ Assert.assertEquals("期望输出值和实际输出值不一致", output, new ScalarReplace("\\d+", "*", true, true).eval(input));
Review Comment:
ok ， I have modified
--
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] BenJFan commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
BenJFan commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r860505628
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/test/java/org/apache/seatunnel/flink/transform/TestReplace.java:
##########
@@ -0,0 +1,48 @@
+package org.apache.seatunnel.flink.transform;
+
+
+import junit.framework.Assert;
+import org.junit.Test;
+
+public class TestReplace {
+
+ /**
+ * Replace the string that the first regular expression matches
+ * Input : Tom's phone number is 123456789 , he's age is 24
+ * Output : Tom's phone number is * , he's age is 24
+ */
+ @Test
+ public void testExample01() {
+ String input = "Tom's phone number is 123456789 , he's age is 24";
+ String output = "Tom's phone number is * , he's age is 24";
+ Assert.assertEquals("期望输出值和实际输出值不一致", output, new ScalarReplace("\\d+", "*", true, true).eval(input));
Review Comment:
Please use English in your code.
--
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] wanghuan2054 commented on a diff in pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
wanghuan2054 commented on code in PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#discussion_r861943329
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/pom.xml:
##########
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>seatunnel-transforms-flink</artifactId>
+ <groupId>org.apache.seatunnel</groupId>
+ <version>2.1.1-SNAPSHOT</version>
Review Comment:
ok, I have modified
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/pom.xml:
##########
@@ -0,0 +1,47 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+ xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
+ <parent>
+ <artifactId>seatunnel-transforms-flink</artifactId>
+ <groupId>org.apache.seatunnel</groupId>
+ <version>2.1.1-SNAPSHOT</version>
+ </parent>
+ <modelVersion>4.0.0</modelVersion>
+
+ <artifactId>seatunnel-transform-flink-replace</artifactId>
+
+ <properties>
+ <junit.version>4.4</junit.version>
Review Comment:
ok, I have modified
##########
seatunnel-transforms/seatunnel-transforms-flink/seatunnel-transform-flink-replace/src/main/java/org/apache/seatunnel/flink/transform/Replace.java:
##########
@@ -0,0 +1,80 @@
+package org.apache.seatunnel.flink.transform;
Review Comment:
ok, I have modified
--
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] ruanwenjun commented on pull request #1761: [Feature][seatunnel-transforms] Add Replace transforms for flink #1759
Posted by GitBox <gi...@apache.org>.
ruanwenjun commented on PR #1761:
URL: https://github.com/apache/incubator-seatunnel/pull/1761#issuecomment-1119209391
FlinkSQL already has built-in replace function, I suggest that we don't need to add this replace transform. What do you think? @CalvinKirs @BenJFan @legendtkl
--
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