Implement Basic SimpleQuery Handling
#505
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #505 +/- ##
==========================================
+ Coverage 60.23% 61.13% +0.89%
==========================================
Files 125 126 +1
Lines 9191 9630 +439
==========================================
+ Hits 5536 5887 +351
- Misses 3655 3743 +88
|
| /// Run a simple text-only query on the Postgres server the connection is connected to. | ||
| /// WARNING: This function is not yet API and is incomplete. | ||
| /// The return type will change to another stream. | ||
| /// | ||
| /// - Parameters: | ||
| /// - query: The simple query to run | ||
| /// - logger: The `Logger` to log into for the query | ||
| /// - file: The file, the query was started in. Used for better error reporting. | ||
| /// - line: The line, the query was started in. Used for better error reporting. | ||
| /// - Returns: A ``PostgresRowSequence`` containing the rows the server sent as the query result. | ||
| /// The sequence be discarded. | ||
| @discardableResult | ||
| public func __simpleQuery( |
There was a problem hiding this comment.
I'm not sure what will happen to this PR but the name is underscored for now so we can merge this PR as-is and I can then create follow-up PRs.
MahdiBM
force-pushed
the
mmbm-simple-query
branch
from
65b21d6
to
5961b27
Compare
|
|
||
| mutating func dataRowReceived(_ dataRow: DataRow) -> Action { | ||
| switch self.state { | ||
| case .rowDescriptionReceived(let queryContext, let columns): |
There was a problem hiding this comment.
Happens for queries with actual rows being returned.
|
|
||
| mutating func commandCompletedReceived(_ commandTag: String) -> Action { | ||
| switch self.state { | ||
| case .messagesSent(let context): |
There was a problem hiding this comment.
Happens for queries that do not return any rows.
| return .succeedQuery(context.promise, with: result) | ||
| } | ||
|
|
||
| case .rowDescriptionReceived(let context, _): |
There was a problem hiding this comment.
Happens for queries that do query some rows to be returned, but no rows matched the query.
| demandStateMachine.receivedRow(dataRow) | ||
| state = .streaming(columns, demandStateMachine) | ||
| let result = QueryResult(value: .rowDescription(columns), logger: queryContext.logger) | ||
| return .succeedQuery(queryContext.promise, with: result) |
There was a problem hiding this comment.
Need to succeed the query here when we receive the first row and state is RowDescription, since we notice that there are rows to be read.
| return self.avoidingStateMachineCoW { state -> Action in | ||
| state = .commandComplete(commandTag: commandTag) | ||
| let result = QueryResult(value: .noRows(.tag(commandTag)), logger: context.logger) | ||
| return .succeedQuery(context.promise, with: result) |
There was a problem hiding this comment.
Need to succeed the query here when we receive the command-complete row and state is RowDescription, since we notice that there are not rows to be read.
| import Logging | ||
| @testable import PostgresNIO | ||
|
|
||
| class SimpleQueryStateMachineTests: XCTestCase { |
There was a problem hiding this comment.
Mostly copy-pasted from extended-query state machine, though properly adjusted to simple-query state machine and the way it should behave.
MahdiBM
force-pushed
the
mmbm-simple-query
branch
from
25a69ec
to
39de666
Compare
MahdiBM
force-pushed
the
mmbm-simple-query
branch
from
35f6d6e
to
26fe020
Compare
|
I wonder if a better way is to build a dedicated |
|
@fabianfett i moved the stuff to that, mid-PR. |
|
FWIW we're already using this function on prod and it's been working as it should. |
Resolves #499
Checklist
SimpleQueryresponse withtextformat.Decoding rows from a simple-query response seems to fail for now, although no-row queries do succeed. I think because as described in Postgres docs, the returned format of data for simple-query istext, and PostgresNIO doesn't properly support that.EDIT: For SimpleQuery only, I removed the line that forced formats to be binary, and things are working now.
I'm not sure how well PostgresNIO supports the text format, but it works for strings and ints.
Not Implemented In This PR
SimpleQuery.