neo4jrb / neo4j-core

A simple unified API that can access both the server and embedded Neo4j database. Used by the neo4j gem
MIT License
99 stars 80 forks source link

Implementing `.where_or` method to `Neo4j::Core::Query` #325

Closed amitsuryavanshi closed 5 years ago

amitsuryavanshi commented 5 years ago

Fixes #

This pull introduces:

eg.

Neo4j::Core::Query.new.where_or(q: {age: 30, name: 'Chunky'})

would generate cypher like WHERE (q.age = {q_age} OR q.name = {q_name})

Pings: @klobuczek

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.3%) to 90.891% when pulling 64428d6214e1dbf9c25da52d7f8f9ec3b642bfc9 on amitsuryavanshi:where_or_cypher into 0ddd20385d334c5b05eefe6e814e2fe44e483a81 on neo4jrb:master.

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.2%) to 90.673% when pulling 51b614b1fb743597618b5165af38a8990a7892a8 on amitsuryavanshi:where_or_cypher into c7388b021350e3a561bfb49ca41074ae5b998e78 on neo4jrb:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.3%) to 90.891% when pulling 64428d6214e1dbf9c25da52d7f8f9ec3b642bfc9 on amitsuryavanshi:where_or_cypher into 0ddd20385d334c5b05eefe6e814e2fe44e483a81 on neo4jrb:master.

coveralls commented 5 years ago

Coverage Status

Coverage increased (+0.3%) to 90.891% when pulling 64428d6214e1dbf9c25da52d7f8f9ec3b642bfc9 on amitsuryavanshi:where_or_cypher into 0ddd20385d334c5b05eefe6e814e2fe44e483a81 on neo4jrb:master.

klobuczek commented 5 years ago

@cheerfulstoic could you have a look at this if you find some time? I remember you talking several times in the past about OR and the difficulties with it. Is this implementation too simplistic? Or what were your hesitations in implementing it the way it is in this PR? Since this expands the DSL I want to be cautious not to introduce something that will be hard to take away once out there.

cheerfulstoic commented 5 years ago

Hrmm, this is probably fine given that there is a where_not method. I think that ActiveRecord uses where.not and where().or.where(), so I think the only complain that I would have is with ActiveRecord compatibility

amitsuryavanshi commented 5 years ago

Closing the as this does not sound like a viable option.