[Java][FlightRPC] FlightSQL error: 'Parameter ordinal out of range' executing a prepared stmt with params

[asfimport]

Hey again :)

I'm getting a 'parameter ordinal 1 out of range' error trying to set a parameter on the returned AvaticaPreparedStatement. Repro:

• Open a FlightSQL JDBC connection

• conn.prepareStatement with a SQL query containing params (e.g. INSERT INTO users (id, name) VALUES (?, ?))

• ps.setString(1, "foo") -> above error, thrown from AvaticaPreparedStatement.getParameter(int)

  I had a bit of a dig to try to identify a potential cause:

• the Meta.Signature passed to the AvaticaPreparedStatement on creation has an empty parameter list - this is what causes the out-of-bounds error.

• in ArrowFlightMetaImpl.prepare, it calls newSignature, but this only takes the SQL query, and so newSignature creates the signature with the empty list. The call to ArrowFlightSqlClientHandler.prepare happens on the line after - could we pass the param Schema from this result to newSignature?

Let me know if I can help narrow this down further or help with the fix :)

James

Reporter: James Henderson / @jarohen
Assignee: David Li / @lidavidm

PRs and other links:

• GitHub Pull Request #14627

_{Note: This issue was originally created as ARROW-18300. Please see the migration documentation for further details.}

[asfimport]

David Li / @lidavidm:
Thanks for the detailed reproduction.

This is going to take me a bit to fix, because it looks like parameters are straight up not implemented (ArrowFlightMetaImpl#execute ignores typedValues). So it'll take me a bit to put this together and ensure it's thoroughly tested.

[asfimport]

James Henderson / @jarohen:
Thanks - yep, no worries, of course.

FWIW we can't return accurate parameter types at the point of creating a prepared statement - partly because XT is a dynamic database, but also partly because we're not sure it's possible in the general case:

• For us, we can't accurately type the params in INSERT INTO users (id, name) VALUES (?, ?) because a user is allowed to put any value in those columns - aware we might be the exception to the rule on this one. We currently say that the param Schema has two columns, both of which are 0-leg dense unions, and then refine it when the user binds some parameters, but I'm not sure how viable that workaround might be long-term.
• More generally, even in statically typed databases, (admittedly quite contrived) queries like SELECT ? FROM foo have untypable params - Postgres, for example, throws an error if you try to get the parameter metadata through JDBC for this one.

[asfimport]

David Li / @lidavidm:
Yeah, and I think that's not been well-defined in the standard. ADBC, for what it's worth, uses NullType/NA to indicate that the parameter type is not known (though this is somewhat abusing the type system), and the SQLite example implementation of Flight SQL returns a dense union type (that's not really feasible if your type system is extensible like Postgres's, though).

@jduo any thoughts here? We should clarify this in the protocol specification.

[asfimport]

David Li / @lidavidm:
I put up a draft PR: #14627

It doesn't yet handle the NullType/NA convention proposed above or any types besides String.

[asfimport]

David Li / @lidavidm:
It looks like I'm not going to get any time to work on this in the near future. Let me know if you want to pick it up.

[avantgardnerio]

If it helps, I think it wouldn't be too hard to get Apache Ballista to return the correct schema to test against. We could definitely use this feature.

AFAICT, this:

arrow/java/flight/flight-sql-jdbc-driver/src/main/java/org/apache/arrow/driver/jdbc/ArrowFlightPreparedStatement.java

Line 74 in af400a8

final Schema resultSetSchema = prepare.getDataSetSchema();

Would need to call this:

arrow/java/flight/flight-sql/src/main/java/org/apache/arrow/flight/sql/FlightSqlClient.java

Line 965 in 17ea6fc

parameterSchema = deserializeSchema(bytes);

And I have some code in a private repo that is providing the schema to be deserialized (DataFusion recently implemented type inference and a method to collect the inferred schema from the parameters)

CC @alamb

[avantgardnerio]

PTAL at #33961

[lidavidm]

@jarohen in case this interests you, @aiguofer pushed this across the finish line! However it currently depends on the server providing accurate types for the parameters, so there's still more that could be improved