Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

GH-37720: [Format][Docs][FlightSQL] Document stateless prepared statements #40243

Merged
merged 12 commits into from
Mar 25, 2024
Merged

GH-37720: [Format][Docs][FlightSQL] Document stateless prepared statements #40243

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
3061d9d
GH-37720: [Format][Docs][FlightSQL] Document FlightSQL stateless prep…
erratic-pattern Feb 13, 2024
cfce010
Fix typo in protobuf docs
erratic-pattern Mar 2, 2024
dc96ef7
Update docs/source/format/FlightSql.rst
erratic-pattern Mar 4, 2024
d4a30c1
Update format/FlightSql.proto
erratic-pattern Mar 4, 2024
0f4d1f4
clarify usage of the updated statement handle
erratic-pattern Mar 4, 2024
bb2d155
make comment style consistent in flightsql proto
erratic-pattern Mar 4, 2024
8ea3ca0
Update format/FlightSql.proto
erratic-pattern Mar 16, 2024
e073001
Update format/FlightSql.proto
erratic-pattern Mar 16, 2024
d4dd713
add additional clarifications about usage of stateless parameters
erratic-pattern Mar 16, 2024
8ddc3c3
clarify legacy behavior for DoPutPreparedResult protobuf message
erratic-pattern Mar 16, 2024
b5ad6c5
Merge branch 'main' of github.com:apache/arrow into adam/flightsql/st…
erratic-pattern Mar 20, 2024
6cffe2f
Update docs/source/format/FlightSql.rst
alamb Mar 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions docs/source/format/FlightSql.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ the ``type`` should be ``ClosePreparedStatement``).
Execute a previously created prepared statement and get the results.

When used with DoPut: binds parameter values to the prepared statement.
The server may optionally respond with an updated handle. The client
should use this updated handle for all subsequent requests for this
prepared statement. The updated handle allows implementing query
parameters with stateless services. Note that the server is responsible
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is very difficult to understand. How does an updated handle allow implementing query parameters? This should be more descriptive (does the handle embody the bound parameters?).

Also, "the client should use this updated handle" means the client cannot bind the original prepared statement to other parameters?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

. How does an updated handle allow implementing query parameters? This should be more descriptive (does the handle embody the bound parameters?).

Maybe we can say something like this:

Optionally updating the handle allows the client to supply all state required to execute the query in an ActionPreparedStatementExecute message. For example, stateless servers can encode the bound parameter values into the new handle, and the client will send that new handle with parameters back to the server.

The problem is as currently written, the CommandPrepareStatementQuery message contains parameter values, but does not return anything to the client prior to the client sending ActionPreparedStatementExecute. Thus the only way to implement prepared statements with bind parameters today is to store the value of the parameters on the server between the call to CommandPrepareStatementQuery and ActionPreparedStatementExecute.

Also, "the client should use this updated handle" means the client cannot bind the original prepared statement to other parameters?

That is an excellent question. I think the intent is that the handle could be updated with new parameter values. I agree as worded this is confusing and should be clarified

Note that a handle returned from a DoPut call with CommandPreparedStatementQuery can itself be passed to a subsequent DoPut call with CommandPreparedStatementQuery to bind a new set of parameters. The subsequent call itself may return an updated handled which again should be used for subsequent requests.

for detecting the case where the client does not use the updated handle on
subsequent requests (older clients may ignore this field) and responding
appropriately.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What does "appropriately" imply? Return an error?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think it means the server should return an error. Perhaps we can reword like

requests (older clients may ignore this field) and returning a clear error.


When used with GetFlightInfo: execute the prepared statement. The
prepared statement can be reused after fetching results.
Expand Down
2 changes: 2 additions & 0 deletions docs/source/format/FlightSql/CommandPreparedStatementQuery.mmd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ Server->>Client: ActionCreatePreparedStatementResult{handle}
loop for each invocation of the prepared statement
Client->>Server: DoPut(CommandPreparedStatementQuery)
Client->>Server: stream of FlightData
Server-->>Client: DoPutPreparedStatementResult{handle}
Note over Client,Server: optional response with updated handle
Client->>Server: GetFlightInfo(CommandPreparedStatementQuery)
Server->>Client: FlightInfo{endpoints: [FlightEndpoint{…}, …]}
loop for each endpoint in FlightInfo.endpoints
Expand Down
Loading
Loading