Skip to content
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

[WIP] Neo4j connector #19154

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from
Draft

Conversation

ragnard
Copy link
Member

@ragnard ragnard commented Sep 25, 2023

Description

Connector for the Neo4j graph database.

Additional context and related issues

See docs for connector overview.

Release notes

[ ] This is not user-visible or is docs only, and no release notes are required.
[ ] Release notes are required. Please propose a release note for me.
[x] Release notes are required, with the following suggested text:

# Neo4j connector
* New connector supporting the Neo4j graph database

@cla-bot
Copy link

cla-bot bot commented Sep 25, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@ragnard ragnard changed the title Neo4j bolt connector Neo4j connector Sep 25, 2023
@github-actions github-actions bot added the docs label Sep 25, 2023
@ragnard ragnard changed the title Neo4j connector [WIP] Neo4j connector Sep 26, 2023
@cla-bot
Copy link

cla-bot bot commented Sep 26, 2023

Thank you for your pull request and welcome to the Trino community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. Continue to work with us on the review and improvements in this PR, and submit the signed CLA to [email protected]. Photos, scans, or digitally-signed PDF files are all suitable. Processing may take a few days. The CLA needs to be on file before we merge your changes. For more information, see https://github.com/trinodb/cla

@cla-bot cla-bot bot added the cla-signed label Sep 26, 2023
@ragnard ragnard force-pushed the neo4j-bolt-connector branch 9 times, most recently from dae9427 to 42a6c3f Compare October 4, 2023 08:38
@ragnard ragnard force-pushed the neo4j-bolt-connector branch 11 times, most recently from 81ebb96 to 2b02a73 Compare October 8, 2023 19:43
@ragnard
Copy link
Member Author

ragnard commented Oct 8, 2023

I'd appreciate an initial review of this. The connector is working as intended.

For tests I've mostly tried to cover the type mappings for the table function, which should cover all types, and the same for the node/relationship tables (although more test cases need to be added here).

@ragnard ragnard requested review from electrum and kokosing and removed request for electrum October 8, 2023 19:46
@ragnard ragnard requested a review from mosabua October 8, 2023 19:47
@ragnard ragnard force-pushed the neo4j-bolt-connector branch 4 times, most recently from 0e30c22 to 161920f Compare October 18, 2023 11:53
@ragnard
Copy link
Member Author

ragnard commented Oct 18, 2023

What are your thoughts on implementing "dynamic tables", like the Pinot connector does?

It would be easy to support queries like:

select * from "match (a:Actor)-[c:ACTED_IN]-(m:Movie) return a, c, m";

ie. the table name is a CQL query.

This would be the similar to functionality provided by the system.query table function that I have currently implemented.
Since Neo4j is dynamically typed there is no way to know the structure of the result.
For the table function the user can optionally provide a DESCRIPTOR for typing the return result, if not, a generic JSON-result is returned (single column of type json named result).
For a dynamic table like the above, we could do the same, so the result would be:

result (json)
{"a": {...}, "c": {...}, "m": {...}}

@martint
Copy link
Member

martint commented Oct 18, 2023

That approach is now discouraged. We used it before we had support for table functions, but it's no longer necessary.

@ragnard
Copy link
Member Author

ragnard commented Oct 18, 2023

That approach is now discouraged. We used it before we had support for table functions, but it's no longer necessary.

Ok, thanks, then I'll stick to the table function.

@ragnard ragnard force-pushed the neo4j-bolt-connector branch 4 times, most recently from 6a9df23 to 33c4020 Compare November 6, 2023 19:48
@ragnard ragnard force-pushed the neo4j-bolt-connector branch 4 times, most recently from cdb4a9b to a954c9d Compare December 3, 2023 13:20
@ragnard ragnard force-pushed the neo4j-bolt-connector branch 3 times, most recently from 0a738d9 to 4d0a813 Compare January 1, 2024 18:20
{
try (Session session = this.driver.session(SessionConfig.forDatabase(SYSTEM_DATABASE))) {
return session.executeRead(tx -> {
return tx.run("SHOW DATABASES YIELD name, type WHERE type <> 'system' RETURN distinct(name)")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to change this query to make it work with the latest version of Trino:
"SHOW DATABASES YIELD name WHERE name <> 'system' RETURN distinct(name)"
Apparently, SHOW DATABASES no longer returns type.

See documentation for usage information.
@ragnard ragnard force-pushed the neo4j-bolt-connector branch from 4d0a813 to de43a71 Compare November 26, 2024 17:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants