rickardoberg / neo4j-jdbc

Neo4j JDBC driver
33 stars 60 forks source link

error with PreparedStatement #7

Open keremkacel opened 11 years ago

keremkacel commented 11 years ago

I'm trying to integrate Neo4J with a product that supports third-party JDBC drivers. But PreparedStatements are failing.

I'm able to reproduce the issue in a Java code snippet.

When I run a hardcoded query via JDBC, it works: START root=node(0) MATCH root-[:a]->company, company-[:b]->account WHERE account.userName = "happy" RETURN account.userName, account.password

But when I run the following Java code with a PreparedStatement, Neo4J gives me a 400 Bad Request.

import java.sql.*;

public class TestDBNeo {
public static void main(String[] args)
throws ClassNotFoundException, SQLException
{
Class.forName("org.neo4j.jdbc.Driver");
String url = "jdbc:neo4j://localhost:7474/";

Connection conn = DriverManager.getConnection(url);

conn.setAutoCommit(false);

String query = "START root=node(0) MATCH root-[:a]->company, company-[:b]->account WHERE account.userName = ? RETURN account.userName, account.password";

PreparedStatement st = null;
st = conn.prepareStatement(query);
st.setString(1, "happy");
ResultSet rset = st.executeQuery(query);

while (rset.next()) {
System.out.println (rset.getString(1));
}

stmt.close();

}
}
Nov 30, 2012 9:43:08 AM org.neo4j.jdbc.Driver createDatabases
INFO: Embedded Neo4j support not enabled org/neo4j/graphdb/GraphDatabaseService
Nov 30, 2012 9:43:08 AM org.neo4j.jdbc.Driver createDatabases
INFO: Embedded Neo4j support not enabled org/neo4j/graphdb/GraphDatabaseService
Connecting to URL http://localhost:7474/
Starting the internal [HTTP/1.1] client
Executing query: START root=node(0) MATCH root-[:a]->company, company-[:b]->account WHERE account.userName = ? RETURN account.userName, account.password
 with params{}
Starting the internal [HTTP/1.1] client
Exception in thread "main" java.sql.SQLException: java.sql.SQLException: Bad Request
        at org.neo4j.jdbc.Neo4jConnection.executeQuery(Neo4jConnection.java:238)
        at org.neo4j.jdbc.Neo4jStatement.execute(Neo4jStatement.java:128)
        at org.neo4j.jdbc.Neo4jStatement.executeQuery(Neo4jStatement.java:44)
        at TestDBNeo.main(TestDBNeo.java:26)
Caused by: java.sql.SQLException: Bad Request
        at org.neo4j.jdbc.rest.RestQueryExecutor.executeQuery(RestQueryExecutor.java:71)
        at org.neo4j.jdbc.Neo4jConnection.executeQuery(Neo4jConnection.java:234)
        ... 3 more
Caused by: Bad Request (400) - Bad Request
        at org.restlet.resource.ClientResource.doError(ClientResource.java:557)
        at org.restlet.resource.ClientResource.handleInbound(ClientResource.java:1130)
        at org.restlet.resource.ClientResource.handle(ClientResource.java:997)
        at org.restlet.resource.ClientResource.handle(ClientResource.java:972)
        at org.restlet.resource.ClientResource.post(ClientResource.java:1381)
        at org.restlet.resource.ClientResource.post(ClientResource.java:1327)
        at org.neo4j.jdbc.rest.RestQueryExecutor.executeQuery(RestQueryExecutor.java:63)
        ... 4 more

I tried escaping the question mark. I tried wrapping the question mark in single quotes, double quotes, etc. Nothing helps.

Thanks,

Kerem

jexp commented 11 years ago

you have to use the cypher parameter syntax which is {1} {2} etc.

see: https://github.com/rickardoberg/neo4j-jdbc/blob/master/src/test/java/org/neo4j/jdbc/Neo4jStatementTest.java#L76

keremkacel commented 11 years ago

Thank you Michael. I just tried it, but I got the same exact error (even the line numbers are the same):

Executing query: START root=node(0) MATCH root-[:a]->company, company-[:b]->account WHERE account.userName = {1} RETURN account.userName, account.password
 with params{}
Starting the internal [HTTP/1.1] client
Exception in thread "main" java.sql.SQLException: java.sql.SQLException: Bad Request
        at org.neo4j.jdbc.Neo4jConnection.executeQuery(Neo4jConnection.java:238)
        at org.neo4j.jdbc.Neo4jStatement.execute(Neo4jStatement.java:128)
        at org.neo4j.jdbc.Neo4jStatement.executeQuery(Neo4jStatement.java:44)
        at TestDBNeo.main(TestDBNeo.java:26)
Caused by: java.sql.SQLException: Bad Request
        at org.neo4j.jdbc.rest.RestQueryExecutor.executeQuery(RestQueryExecutor.java:71)
        at org.neo4j.jdbc.Neo4jConnection.executeQuery(Neo4jConnection.java:234)
        ... 3 more
Caused by: Bad Request (400) - Bad Request
        at org.restlet.resource.ClientResource.doError(ClientResource.java:557)
        at org.restlet.resource.ClientResource.handleInbound(ClientResource.java:1130)
        at org.restlet.resource.ClientResource.handle(ClientResource.java:997)
        at org.restlet.resource.ClientResource.handle(ClientResource.java:972)
        at org.restlet.resource.ClientResource.post(ClientResource.java:1381)
        at org.restlet.resource.ClientResource.post(ClientResource.java:1327)
        at org.neo4j.jdbc.rest.RestQueryExecutor.executeQuery(RestQueryExecutor.java:63)
        ... 4 more
wargre commented 11 years ago

Hello,

This is not an issue, but a bug in your code :

PreparedStatement st = null; st = conn.prepareStatement(query); st.setString(1, "happy"); ResultSet rset = st.executeQuery(query); <<<<<<================= executeQuery(); without parameter work better

This issue should be closed.

keremkacel commented 11 years ago

Thank you wargre, you were right about the bug in my code. However, some applications convert substitutions in JDBC queries to question marks, regardless of language.

For example, Vordel API Server lets you define your own query for authentication via JDBC. Example:

START root = node(0)
MATCH root-[:RootHasServiceAccount]->sa
WHERE sa.username = ${username}
RETURN sa.username, sa.password

where ${username} is a variable to be evaluated using Java EL.

Before the query is executed, it's converted to a PreparedStatement where the query is:

START root = node(0)
MATCH root-[:RootHasServiceAccount]->sa
WHERE sa.username = ?
RETURN sa.username, sa.password

and this ? is to be populated using setString() with the value of the ${username} variable.

I was able to solve my problem by modifying the executeQuery function to convert every ? to {1} {2} etc. Note: I'm aware that a ? directly after an alphanumeric character (e.g. n.name?) is legal Cypher syntax and is not supposed to be converted.

I'm not sure if ? is a standard practice in supporting third-party JDBC libraries, but I'm sure there are some other applications that implement substitutions with a ?. Let me know if this warrants a patch, and I'll submit my code.

Kerem

wargre commented 11 years ago

Replacing text in query can be dangerous as it need a complete query parsing. Ex:

START n=node:myIndex(name=?) 
 MATCH n-[t?]->s 
 WHERE t.name?~="^[a-c]?[0-9]*" and s.desc="what\"=? that" 
 WITH s 
 CREATE UNIQUE s-[:test{val=?}]->p 
 SET s.val=?

You need to parse carefully the query to detect the '?' that are parameters. In short you need to parse the query like neo4j server will do... before neo4j redo this parsing... IMHO it not worth it. (complex / resource consuming )

Can you trick the system by writing this query :

START root = node(0)
MATCH root-[:RootHasServiceAccount]->sa
WHERE sa.username${username} = {1}
RETURN sa.username, sa.password
keremkacel commented 11 years ago

Thanks wargre.

If I understand the Cypher syntax correctly, all I need to ignore is: i) a ? within single or double quotes (but consider escaped quote characters may exist within quotes) ii) a ? directly after a node/rel/property name Is there anything I missed?

I wrote a relatively simple algorithm for this that runs in O(n) time (counts the characters one by one).

About the query suggestion you posted... That's a good idea, but it results in: sa.username? = {1} and an equality with a question mark (rather than an exclamation mark) would match some unwanted nodes where the property is null.

I guess I could use the following for a hack:

START root = node(0)
MATCH root-[:RootHasServiceAccount]->sa
WHERE sa.zzz${username} = '' AND sa.username = {1}
RETURN sa.username, sa.password

which would result in:

START root = node(0)
MATCH root-[:RootHasServiceAccount]->sa
WHERE sa.zzz? = '' AND sa.username = {1}
RETURN sa.username, sa.password

and return the correct set of records, as long as sa never has a property named zzz.

Kerem