Neo4jError: LIMIT: Invalid input. '10.0' is not a valid value. Must be a non-negative integer.

Exide commented 4 years ago

Neo4j Version: 4.0.1 Enterprise Neo4j Mode: Casual cluster with 3 cores, 1 read-replica Driver version: JavaScript driver 4.0.2 Operating System: Ubuntu 18.10 on AWS

Steps to reproduce

Start Neo4j (version 4.0.x)

Run the following query (driver version 4.0.0, 4.0.1, or 4.0.2)

await session.run('MATCH (n) RETURN n LIMIT $limit', { limit: 10 });

Expected behavior

The query successfully completes, returning up to 10 nodes.

Actual behavior

The session.run call throws the following error:

{ Neo4jError: LIMIT: Invalid input. '10.0' is not a valid value. Must be a non-negative integer.

    at captureStacktrace (/Users/exide/sandbox/node_modules/neo4j-driver/lib/result.js:263:15)
    at new Result (/Users/exide/sandbox/node_modules/neo4j-driver/lib/result.js:68:19)
    at Session._run (/Users/exide/sandbox/node_modules/neo4j-driver/lib/session.js:174:14)
    at Session.run (/Users/exide/sandbox/node_modules/neo4j-driver/lib/session.js:135:19)
    at /Users/exide/sandbox/neo4j-limit-bug.js:21:23
    at processTicksAndRejections (internal/process/next_tick.js:81:5)
  code: 'Neo.ClientError.Statement.ArgumentError',
  name: 'Neo4jError' }

zhenlineo commented 4 years ago

Hi @Exide, Could you try to follow the instructions here to provide int values and retry your query? https://github.com/neo4j/neo4j-javascript-driver#writing-integers

Exide commented 4 years ago

Certainly!

I've updated the query to be:

await session.run('MATCH (n) RETURN n LIMIT $limit', { limit: neo4j.int(10) });

This worked without warnings/errors. Is this the recommended approach going forward and the docs just need an update or should I expect a bug fix at some point in the future?

zhenlineo commented 4 years ago

Hi @Exide,

Could you explain a bit more about which docs that you think need to be updated? Could you be more explicit about your suggestions? :)

Exide commented 4 years ago

@zhenlineo Am I to understand this isn't a bug? If so, I find that unfortunate.

My suggestion would then be to update the README's section on breaking changes to include this detail.

When using the 1.7.x driver the following syntax is valid:

session.run('MATCH (n) RETURN n LIMIT $limit', { limit: 10 });

When running that same query with the 4.x driver a Neo4jError is thrown. You are now required to cast your integer to an integer, using neo4j.int.

zhenlineo commented 4 years ago

Hi @Exide,

Thanks for your explanation. I did not expect it actually did not fail on 1.7. neo4j.int(10) is always our suggested way to provide int number for this driver.

We will keep you updated what we shall do with the 1.7 unexpected behaviour.

Cheers, Zhen

zhenlineo commented 4 years ago

Hi @Exide,

I just run some tests. And I've verified that even with 1.7 driver, if I run the test against a 4.0 server, then I will get the same error as 4.0 driver. So it is clear that this is caused by some changes in cypher engine in 4.0 neo4j server. I guess the cypher engine now checks more strictly to ensure that the number for LIMIT has to be an integer. Maybe in 3.5 this number can be a float, thus why the query can be successful before.

As for the new Cypher grammar, I personally feel this integer check is valid for number given to LIMIT. If you have more questions about the cypher, I suggest you to open an issue on Neo4j repo instead. For the driver, we did not see a bug that we should fix.

Cheers, Zhen

Exide commented 4 years ago

I ran the same test (1.7.6 driver against an out-of-the-box 4.0.1 server) and I did not encounter any errors. That is what led me to believe this was a bug. Regardless of that though, understanding that the intent was for neo4j.int to always be required gives me the info I need to move forward :)

Thanks @zhenlineo for the quick responses.

abdelhak-ajbouni commented 3 years ago

Hi guys I usually use something like this

session.run( `MATCH (n) RETURN n LIMIT ${limit}` );

and it works. not sure if it has any drawbacks tho

Marius2m commented 3 years ago

Hi guys I usually use something like this
session.run( `MATCH (n) RETURN n LIMIT ${limit}` );
and it works. not sure if it has any drawbacks tho

It might not be safe to use it like that. Only do it when your data is 100% checked and sanitised.

Passing query params as 2nd param from .run(query, queryParams) should give protection against 'SQL injection' issues. Honestly, I don't know much about these attacks on cypher, but I assume that on APOC procedures it might happen, so don't use string interpolation if you can.

neo4j / neo4j-javascript-driver