Add Ignite connector #8323

chenjian2664 · 2021-06-19T18:15:50Z

Fixes #8098

Add a connector for Apache Ignite.

docs/src/main/sphinx/connector/ignite.rst

plugin/trino-ignite/pom.xml

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteContainer.java

docs/src/main/sphinx/connector/ignite.rst

plugin/trino-ignite/pom.xml

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteContainer.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/pom.xml

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteJdbcPlugin.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteContainer.java

pom.xml

hashhar

Some initial comments in addition to existing ones

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/pom.xml

hashhar

Thanks @chenjian2664 for working on this. This looks much better than expected for the initial version of a connector.

Some initial comments. Haven't taken a look at the test classes yet. Will do so later.

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcClient.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/ImplementAvgDecimal.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestingIgniteServer.java

docs/src/main/sphinx/connector/ignite.rst

hashhar · 2021-08-27T11:58:38Z

Thanks for picking this up again @chenjian2664 ❤️

Looks like there are some conflicts now.
Can you rebase your branch against latest master from Trino?

chenjian2664 · 2021-08-30T11:33:16Z

@hashhar Thanks. Can you help to look at it again? :)

hashhar

Sorry for the delay in getting to this @chenjian2664.

Some initial comments.
Main ones are about some overrides in IgniteJdbcClient and the IgniteQueryRunner.
Will do another pass once those are addressed.

plugin/trino-base-jdbc/src/test/java/io/trino/plugin/jdbc/BaseJdbcConnectorTest.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcClient.java

docs/src/main/sphinx/connector/ignite.rst

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/IgniteQueryRunner.java

chenjian2664

#8323 (comment)
@hashhar
Ignite not support CATS, so I must create table first then process a transactional insert here.

mosabua

From the docs perspective this is definitely good enough to merge. Nice job @chenjian2664 !

chenjian2664 · 2023-02-09T17:19:58Z

cc @martint @electrum

kokosing

I am concerned about amount of code that is copied from jdbc connector. Can we try to reuse more code?

.github/workflows/ci.yml

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClientModule.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteConnectorFactory.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteContainer.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestingIgniteServer.java

kokosing

% last comment.

I will merge it after the release. Let's give some time to others to take a look if they like.

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestingIgniteServer.java

bitsondatadev · 2023-02-15T16:45:27Z

Amazing work here @chenjian2664! Would you like to join the Trino Community Broadcast some time after this gets merged?

chenjian2664 · 2023-02-15T22:53:45Z

Amazing work here @chenjian2664! Would you like to join the Trino Community Broadcast some time after this gets merged?

Yes, of course!

ebyhr

Most comments are minor, but I strongly recommend adding char/varchar type mapping tests to this PR.

Also, please remove a redundant commit body. It's the same as the commit title.

plugin/trino-ignite/pom.xml

docs/src/main/sphinx/connector/ignite.rst

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteSqlTypeMapping.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestingIgniteServer.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

kokosing

Please squash the commit and rebase to new release.

core/trino-server/src/main/provisio/trino.xml

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteJdbcMetadataFactory.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClientModule.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteSqlTypeMapping.java

pom.xml

...uncher/src/main/java/io/trino/tests/product/launcher/env/environment/EnvMultinodeIgnite.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteSqlTypeMapping.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestingIgniteContainer.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/pom.xml

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

chenjian2664 · 2023-02-21T21:27:52Z

web-ui-checks hit #16179

ebyhr

Looks good except for comments.

ebyhr · 2023-02-22T02:07:43Z

plugin/trino-ignite/pom.xml

+        <dependency>
+            <groupId>org.apache.ignite</groupId>
+            <artifactId>ignite-core</artifactId>
+            <exclusions>


plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteTypeMapping.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClientModule.java

ebyhr · 2023-02-22T09:56:38Z

@chenjian2664 Thank you so much for working on this patiently!

Let me leave the final merge to @kokosing

kokosing · 2023-02-22T10:00:43Z

Thank you @ebyhr for stepping in.

@chenjian2664 Please rebase and squash commits, once CI get green I will merge it. Thank you for collaboration! Great work!

chenjian2664 · 2023-02-22T10:11:32Z

Thank you for your patient guidance and professional review, without you I would not be able to do such an amazing work.
Thank you all for the great help !!
@kokosing @ebyhr @hashhar @findepi @mosabua

cla-bot bot added the cla-signed label Jun 19, 2021

chenjian2664 self-assigned this Jun 19, 2021

chenjian2664 requested review from findepi, electrum, martint and kokosing June 19, 2021 18:16

tooptoop4 mentioned this pull request Jun 20, 2021

More https://trino.io/resources.html #3472

Closed

chenjian2664 marked this pull request as draft June 20, 2021 05:34

chenjian2664 marked this pull request as ready for review June 20, 2021 08:03

kokosing reviewed Jun 22, 2021

View reviewed changes

chenjian2664 marked this pull request as draft June 24, 2021 02:38

kokosing reviewed Jun 24, 2021

View reviewed changes

findepi reviewed Jun 24, 2021

View reviewed changes

hashhar reviewed Jun 28, 2021

View reviewed changes

chenjian2664 mentioned this pull request Jun 30, 2021

Support Time and Timestamp type for Apache Ignite #8433

Open

chenjian2664 marked this pull request as ready for review July 5, 2021 06:36

hashhar reviewed Jul 8, 2021

View reviewed changes

chenjian2664 mentioned this pull request Jul 14, 2021

Support user customs schema in Ignite connector #8552

Open

mosabua requested changes Jul 15, 2021

View reviewed changes

chenjian2664 requested a review from mosabua July 19, 2021 02:46

findepi force-pushed the master branch from 8538e49 to 1f896ea Compare July 30, 2021 22:13

chenjian2664 requested a review from hashhar August 27, 2021 06:14

hashhar reviewed Sep 5, 2021

View reviewed changes

chenjian2664 commented Sep 7, 2021

View reviewed changes

martint added the roadmap Top level issues for major efforts in the project label Sep 21, 2021

chenjian2664 marked this pull request as draft November 3, 2021 06:09

chenjian2664 marked this pull request as ready for review November 14, 2021 11:59

chenjian2664 requested a review from hashhar November 15, 2021 07:03

mosabua approved these changes Feb 7, 2023

View reviewed changes

kokosing reviewed Feb 10, 2023

View reviewed changes

kokosing approved these changes Feb 15, 2023

View reviewed changes

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestingIgniteServer.java Outdated Show resolved Hide resolved

ebyhr reviewed Feb 17, 2023

View reviewed changes

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java Outdated Show resolved Hide resolved

ebyhr reviewed Feb 17, 2023

View reviewed changes

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java Outdated Show resolved Hide resolved

ebyhr mentioned this pull request Feb 17, 2023

Add Ignite Jdbc config #16148

Closed

kokosing reviewed Feb 17, 2023

View reviewed changes

core/trino-server/src/main/provisio/trino.xml Outdated Show resolved Hide resolved

ebyhr requested changes Feb 20, 2023

View reviewed changes

ebyhr reviewed Feb 20, 2023

View reviewed changes

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteMetadata.java Outdated Show resolved Hide resolved

plugin/trino-ignite/src/test/java/io/trino/plugin/ignite/TestIgniteConnectorTest.java Outdated Show resolved Hide resolved

chenjian2664 requested a review from ebyhr February 21, 2023 05:08

ebyhr requested changes Feb 21, 2023

View reviewed changes

chenjian2664 requested a review from ebyhr February 21, 2023 08:56

ebyhr reviewed Feb 22, 2023

View reviewed changes

This was referenced Feb 22, 2023

Support Trino CTAS data setup process for Ignite connector #16214

Closed

Make sure Ignite primary_key column exists while creating table #16215

Closed

ebyhr approved these changes Feb 22, 2023

View reviewed changes

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClient.java Outdated Show resolved Hide resolved

plugin/trino-ignite/src/main/java/io/trino/plugin/ignite/IgniteClientModule.java Outdated Show resolved Hide resolved

Add Ignite connector

74694cf

martint merged commit b5ef0ac into trinodb:master Feb 22, 2023

github-actions bot added this to the 408 milestone Feb 22, 2023

colebow mentioned this pull request Feb 23, 2023

Add Trino 408 release notes #16209

Merged

chenjian2664 deleted the add_ignite_connector branch February 24, 2023 06:47

ebyhr mentioned this pull request Aug 7, 2023

Add Exasol connector #16202

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Ignite connector #8323

Add Ignite connector #8323

chenjian2664 commented Jun 19, 2021 •

edited by findepi

Loading

hashhar left a comment

hashhar left a comment

hashhar commented Aug 27, 2021

chenjian2664 commented Aug 30, 2021

hashhar left a comment •

edited

Loading

chenjian2664 left a comment

mosabua left a comment

chenjian2664 commented Feb 9, 2023

kokosing left a comment

kokosing left a comment

bitsondatadev commented Feb 15, 2023

chenjian2664 commented Feb 15, 2023

ebyhr left a comment

kokosing left a comment

chenjian2664 commented Feb 21, 2023 •

edited

Loading

ebyhr left a comment

ebyhr Feb 22, 2023

ebyhr commented Feb 22, 2023

kokosing commented Feb 22, 2023

chenjian2664 commented Feb 22, 2023

Add Ignite connector #8323

Add Ignite connector #8323

Conversation

chenjian2664 commented Jun 19, 2021 • edited by findepi Loading

hashhar left a comment

Choose a reason for hiding this comment

hashhar left a comment

Choose a reason for hiding this comment

hashhar commented Aug 27, 2021

chenjian2664 commented Aug 30, 2021

hashhar left a comment • edited Loading

Choose a reason for hiding this comment

chenjian2664 left a comment

Choose a reason for hiding this comment

mosabua left a comment

Choose a reason for hiding this comment

chenjian2664 commented Feb 9, 2023

kokosing left a comment

Choose a reason for hiding this comment

kokosing left a comment

Choose a reason for hiding this comment

bitsondatadev commented Feb 15, 2023

chenjian2664 commented Feb 15, 2023

ebyhr left a comment

Choose a reason for hiding this comment

kokosing left a comment

Choose a reason for hiding this comment

chenjian2664 commented Feb 21, 2023 • edited Loading

ebyhr left a comment

Choose a reason for hiding this comment

ebyhr Feb 22, 2023

Choose a reason for hiding this comment

ebyhr commented Feb 22, 2023

kokosing commented Feb 22, 2023

chenjian2664 commented Feb 22, 2023

chenjian2664 commented Jun 19, 2021 •

edited by findepi

Loading

hashhar left a comment •

edited

Loading

chenjian2664 commented Feb 21, 2023 •

edited

Loading