-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Feature][Connector-V2] add tdengine source #2832
Conversation
Hi, please solve ci problem and conflict. Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you add e2e testcase?
new version e2e:
https://github.com/apache/incubator-seatunnel/tree/dev/seatunnel-e2e/seatunnel-connector-v2-e2e
OR
old version e2e:
https://github.com/apache/incubator-seatunnel/tree/dev/seatunnel-e2e/seatunnel-flink-connector-v2-e2e/connector-iotdb-flink-e2e
https://github.com/apache/incubator-seatunnel/tree/dev/seatunnel-e2e/seatunnel-spark-connector-v2-e2e/connector-iotdb-spark-e2e
...gine/src/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSink.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceReader.java
Outdated
Show resolved
Hide resolved
...org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceSplitEnumerator.java
Show resolved
Hide resolved
...org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceSplitEnumerator.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSinkWriter.java
Show resolved
Hide resolved
081314e
to
0048e00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pre-version is PIW version. Now It's completed.
Please fix the code style. |
130e0f4
to
f37d682
Compare
seatunnel-examples/seatunnel-flink-connector-v2-example/pom.xml
Outdated
Show resolved
Hide resolved
...s/seatunnel-flink-connector-v2-example/src/main/resources/examples/tdengine_to_tdengine.conf
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSinkWriter.java
Outdated
Show resolved
Hide resolved
+ " ) VALUES (" | ||
+ StringUtils.join(convertDataType(metrics), ",") | ||
+ ");"; | ||
final int rowCount = statement.executeUpdate(sql); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you use batch flush
for jdbc?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
org.apache.seatunnel.connectors.seatunnel.tdengine.sink.TDengineSinkWriter#write
method has only one SeaTunnelRow
param, so I think batch flush
will be used when we have write(List<SeaTunnelRow>)
api.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether to support write(List<SeaTunnelRow>)
needs to be discussed. Currently ST only supports write (SeaTunnelRow), if you need better performance, you need to implement batch flush yourself.
...rc/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSinkWriter.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSinkWriter.java
Outdated
Show resolved
Hide resolved
Thx for reminding me . I will do this later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- add sink docs
- revert example module changes
- add e2e testcase
seatunnel-examples/seatunnel-flink-connector-v2-example/pom.xml
Outdated
Show resolved
Hide resolved
Retry CI |
608e4be
to
2e18941
Compare
@EricJoy2048 @hailin0 @Hisoka-X @TyrantLucifer @CalvinKirs |
I can't find the previous context, why can't we implement this in |
The reason why we implement tdengine connector separately is that there are some distinctive concepts in TDengine that do not belong to JDBC, such as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seatunnel-api/src/main/java/org/apache/seatunnel/api/table/type/SqlType.java
Outdated
Show resolved
Hide resolved
seatunnel-examples/seatunnel-flink-connector-v2-example/pom.xml
Outdated
Show resolved
Hide resolved
...use-e2e/src/test/java/org/apache/seatunnel/connectors/seatunnel/clickhouse/ClickhouseIT.java
Outdated
Show resolved
Hide resolved
18cfcdb
to
12bf101
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
+ " ) VALUES (" | ||
+ StringUtils.join(convertDataType(metrics), ",") | ||
+ ");"; | ||
final int rowCount = statement.executeUpdate(sql); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whether to support write(List<SeaTunnelRow>)
needs to be discussed. Currently ST only supports write (SeaTunnelRow), if you need better performance, you need to implement batch flush yourself.
...ain/java/org/apache/seatunnel/connectors/seatunnel/tdengine/config/TDengineSourceConfig.java
Show resolved
Hide resolved
...ain/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceReader.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
...org/apache/seatunnel/connectors/seatunnel/tdengine/exception/TDengineConnectorErrorCode.java
Outdated
Show resolved
Hide resolved
...rc/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/sink/TDengineSinkWriter.java
Outdated
Show resolved
Hide resolved
.../src/main/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSource.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceReader.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceReader.java
Outdated
Show resolved
Hide resolved
...ain/java/org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceReader.java
Outdated
Show resolved
Hide resolved
...org/apache/seatunnel/connectors/seatunnel/tdengine/source/TDengineSourceSplitEnumerator.java
Outdated
Show resolved
Hide resolved
...n/java/org/apache/seatunnel/connectors/seatunnel/tdengine/typemapper/TDengineTypeMapper.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* [Feature][Connector-V2]add tdengine source and sink 1. add ResultSet.TYPE_FORWARD_ONLY, ResultSet.CONCUR_READ_ONLY to statement 2. add tdengine e2e module * SourceSplitEnumerator.Context Co-authored-by: bjyflihongyu <lihongyuinfo@jd.com> Co-authored-by: tyrantlucifer <tyrantlucifer@gmail.com>
Purpose of this pull request
refer to #2671
Check list
New License Guide