jamesfer / cypher-query-builder

An flexible and intuitive query builder for Neo4j and Cypher.
http://jamesfer.me/cypher-query-builder/index.html
MIT License
104 stars 22 forks source link

Add session parameters #161

Open bbenzikry opened 3 years ago

bbenzikry commented 3 years ago

Hi there, we're testing out the library against a causal cluster with multiple database support.
We didn't find a proper way ( let me know if we just missed something ) to pass session parameters from the basic connection instance - so we currently inherit and add an implementation similar to the one in the PR.

Unfortunately I don't really have the time to add tests etc. for the addition and different session configuration values, but thought it would be a good addition in any case, especially for enterprise users.

databu commented 3 years ago

I'm using a workaround for that: I don't call .run() on the builder but call .buildQueryObject() to get the query object instead, which one can then use to run in a neo4j-driver-created session. That way, one can also use transactions, and generally the driver API.

This is better anyway IMHO, then the query builder can focus on its raison d'être -- building the query -- and doesn't have to meddle with the connection, session and transaction handling, error cases therein etc. Good old separation of concerns.

Example:

const query = new Query().create(...).buildQueryObject();
const session = neo4jDriver.session({ database: database });
return session.writeTransaction(tx => { 
  return tx.run(query.query, query.params)
    .then(...);
}).finally(() => session.close());
bbenzikry commented 3 years ago

Hi @databu thanks for replying. I'll start by saying we also resorted to using a similar approach ( for constructed queries ) to the one above for transactions specifically. However, I don't necessarily agree it's the main appeal - having a single fluent API to work with for both raw / cypher and constructed queries is also a factor. Separation is nice, but so is abstraction and DX.

bboure commented 3 years ago

I'd like to back this PR, although I would move the session params at the Query level (instead of the Connection level). That leaves you more flexibility to choose not only the DB, but also the mode (READ/WRITE) depending on your query.

bbenzikry commented 3 years ago

Hi @bboure, thanks for responding - I think what you mentioned deserves a separate PR for query level overrides. When we use a lot of queries that are pretty similar, we prefer the connection and session to have sensible and configurable defaults. In our case, we pass a dataloader between graphql resolvers and having the session preconfigured to the right mode / db etc. completely abstracts away handling properties that are not related to logic IMHO.