diff --git a/CHANGELOG.md b/CHANGELOG.md --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,10 @@ +# v1.0.17.0 + +## Non-breaking + +- Added a `getvalue` spec (`Pqi.Conformance.Operation.Getvalue.ManyRows`) that scans 200,000 single-column rows with `getisnull`, `getlength` and `getvalue` under a 5 s deadline, checking every cell. The divergence is in cost rather than in any returned value, so the observation is whether the scan finished. `libpq` finishes in about 0.1 s. Found `pqi-native` reaching each row by walking its row list from the head, which makes a scan quadratic in the number of rows, so it ran past the deadline. Found via `pqi-native` issue #15. +- Added `execParams` and `exec` specs (`Pqi.Conformance.Operation.ExecParams.Interrupted`, `Pqi.Conformance.Operation.Exec.Interrupted`) covering a connection that is reused while results of an earlier query are still unread: a slow query interrupted by `timeout`, and a `sendQuery` or `sendQueryParams` whose results are never fetched. They observe the results of the synchronous calls that follow, what `getResult` still returns, and the transaction status. Whether `timeout` returns early is not compared, since `libpq`'s blocking call cannot be interrupted. `libpq` discards leftover results at the start of each synchronous call (`PQexecStart`), so each call gets its own result. Found `pqi-native` returning the earlier query's result instead, so that each later call returned the result of the one before it. Found via `pqi-native` issue #16. + # v1.0.16.1 Doc corrections: the README and package description describe adapters as `Pqi.Adapter` values instead of `Pqi.IsConnection` instances, and use the `notifyBePid` field name. diff --git a/pqi-conformance.cabal b/pqi-conformance.cabal --- a/pqi-conformance.cabal +++ b/pqi-conformance.cabal @@ -1,6 +1,6 @@ cabal-version: 3.0 name: pqi-conformance -version: 1.0.16.1 +version: 1.0.17.0 category: Database, PostgreSQL, Testing synopsis: Differential conformance tests for pqi adapters description: @@ -118,8 +118,10 @@ Pqi.Conformance.Operation.EscapeStringConn Pqi.Conformance.Operation.Exec Pqi.Conformance.Operation.Exec.ConnectionLostMidQuery + Pqi.Conformance.Operation.Exec.Interrupted Pqi.Conformance.Operation.Exec.LostConnection Pqi.Conformance.Operation.ExecParams + Pqi.Conformance.Operation.ExecParams.Interrupted Pqi.Conformance.Operation.ExecParams.LargeField Pqi.Conformance.Operation.ExecParams.LostConnection Pqi.Conformance.Operation.ExecPrepared @@ -143,6 +145,7 @@ Pqi.Conformance.Operation.GetResult Pqi.Conformance.Operation.GetResult.LostConnection Pqi.Conformance.Operation.Getvalue + Pqi.Conformance.Operation.Getvalue.ManyRows Pqi.Conformance.Operation.GetvalueCopy Pqi.Conformance.Operation.Host Pqi.Conformance.Operation.IsBusy diff --git a/src/library/Pqi/Conformance/Operation/Exec.hs b/src/library/Pqi/Conformance/Operation/Exec.hs --- a/src/library/Pqi/Conformance/Operation/Exec.hs +++ b/src/library/Pqi/Conformance/Operation/Exec.hs @@ -8,6 +8,7 @@ import qualified Pqi import Pqi.Conformance.Harness import qualified Pqi.Conformance.Operation.Exec.ConnectionLostMidQuery as ConnectionLostMidQuery +import qualified Pqi.Conformance.Operation.Exec.Interrupted as Interrupted import qualified Pqi.Conformance.Operation.Exec.LostConnection as LostConnection import Pqi.Conformance.Prelude import Pqi.Conformance.Scenario @@ -16,6 +17,7 @@ spec :: Pqi.Adapter -> SpecWith ByteString spec adapter = do ConnectionLostMidQuery.spec adapter + Interrupted.spec adapter LostConnection.spec adapter describe "exec" do diff --git a/src/library/Pqi/Conformance/Operation/Exec/Interrupted.hs b/src/library/Pqi/Conformance/Operation/Exec/Interrupted.hs new file mode 100644 --- /dev/null +++ b/src/library/Pqi/Conformance/Operation/Exec/Interrupted.hs @@ -0,0 +1,65 @@ +-- | Coverage for a connection reused after its results were left unread: a +-- slow 'Pqi.exec' cut short by 'timeout', and a 'Pqi.sendQuery' or +-- 'Pqi.sendQueryParams' whose results are never fetched. Each is followed by +-- synchronous calls on the same connection. +-- +-- The divergence: @pqi-native@'s synchronous flows sent their query and read +-- up to the first @ReadyForQuery@, which belonged to the earlier query +-- whenever its messages were still unread. The call then returned the +-- earlier query's result, every call after it the result of the one before, +-- and 'Pqi.getResult' afterwards the last call's. @libpq@ discards any +-- results left over from a previous query at the start of each synchronous +-- call (@PQexecStart@ in @fe-exec.c@), and its @PQexec@ cannot be +-- interrupted at all (it is a blocking foreign call, so the exception is +-- delivered once it returns), so each call gets its own result. +-- +-- Where it surfaced: @pqi-native@ issue #16, which reported the interrupted +-- case for 'Pqi.execParams' (covered by +-- 'Pqi.Conformance.Operation.ExecParams.Interrupted'). +-- +-- Why this observation: whether 'timeout' returns early differs by design +-- (the reference cannot be interrupted), so only what the connection does +-- afterwards is compared. +module Pqi.Conformance.Operation.Exec.Interrupted + ( spec, + ) +where + +import qualified Pqi +import Pqi.Conformance.Harness +import Pqi.Conformance.Observation +import Pqi.Conformance.Prelude +import Pqi.Conformance.Scenario +import System.Timeout (timeout) +import Test.Hspec + +spec :: Pqi.Adapter -> SpecWith ByteString +spec adapter = + describe "exec" do + describe "after unread results" do + it "of an interrupted exec, the next calls return their own results" \conninfo -> + differential adapter conninfo \connection -> do + _ <- timeout 200_000 (Pqi.exec connection "select 'slow' from pg_sleep (1)") + afterwards connection + it "of a sendQuery, the next calls return their own results" \conninfo -> + differential adapter conninfo \connection -> do + sent <- Pqi.sendQuery connection "select 'pending'" + (sent,) <$> afterwards connection + it "of a sendQueryParams, execParams returns its own result" \conninfo -> + differential adapter conninfo \connection -> do + sent <- Pqi.sendQueryParams connection "select 'pending'" [] Pqi.Text + next <- observed "select 'next'" [] Pqi.Text connection + remaining <- takeResult connection + pure (sent, next, remaining) + +-- | Two synchronous calls, then whatever 'Pqi.getResult' still has, and the +-- transaction status. +afterwards :: + Pqi.Connection -> + IO (Maybe ResultObservation, Maybe ResultObservation, Maybe ResultObservation, Pqi.TransactionStatus) +afterwards connection = do + one <- execScenario "select 'one'" connection + two <- execScenario "select 'two'" connection + remaining <- takeResult connection + transactionStatus <- Pqi.transactionStatus connection + pure (one, two, remaining, transactionStatus) diff --git a/src/library/Pqi/Conformance/Operation/ExecParams.hs b/src/library/Pqi/Conformance/Operation/ExecParams.hs --- a/src/library/Pqi/Conformance/Operation/ExecParams.hs +++ b/src/library/Pqi/Conformance/Operation/ExecParams.hs @@ -9,6 +9,7 @@ import qualified Pqi as Lq import Pqi.Conformance.Harness import Pqi.Conformance.Observation +import qualified Pqi.Conformance.Operation.ExecParams.Interrupted as Interrupted import qualified Pqi.Conformance.Operation.ExecParams.LargeField as LargeField import qualified Pqi.Conformance.Operation.ExecParams.LostConnection as LostConnection import Pqi.Conformance.Prelude @@ -17,6 +18,7 @@ spec :: Lq.Adapter -> SpecWith ByteString spec adapter = do + Interrupted.spec adapter LargeField.spec adapter LostConnection.spec adapter diff --git a/src/library/Pqi/Conformance/Operation/ExecParams/Interrupted.hs b/src/library/Pqi/Conformance/Operation/ExecParams/Interrupted.hs new file mode 100644 --- /dev/null +++ b/src/library/Pqi/Conformance/Operation/ExecParams/Interrupted.hs @@ -0,0 +1,49 @@ +-- | Coverage for a connection reused after an asynchronous exception +-- interrupted 'Pqi.execParams' while it waited for its results: a slow +-- query is cut short by 'timeout', then two quick queries run on the same +-- connection. +-- +-- The divergence: @pqi-native@'s synchronous flows sent the query and read +-- up to the next @ReadyForQuery@ without recording that a query was in +-- flight. Once the read was interrupted, the slow query's messages stayed +-- unread, so the next call read them as its own and returned the slow +-- query's result, and every call after that returned the result of the one +-- before it, with no error. @libpq@ cannot be interrupted there at all +-- (@PQexecParams@ is a blocking foreign call, so the exception is delivered +-- once it returns), and the next call gets its own result. @libpq@ also +-- discards any results left over from a previous query at the start of each +-- synchronous call (@PQexecStart@ in @fe-exec.c@). +-- +-- Where it surfaced: @pqi-native@ issue #16. +-- +-- Why this observation: whether 'timeout' returns early differs by design +-- (the reference cannot be interrupted), so only what the connection does +-- afterwards is compared: the results of the two later queries and the +-- transaction status. +module Pqi.Conformance.Operation.ExecParams.Interrupted + ( spec, + ) +where + +import qualified Pqi +import Pqi.Conformance.Harness +import Pqi.Conformance.Observation +import Pqi.Conformance.Prelude +import Pqi.Conformance.Scenario +import System.Timeout (timeout) +import Test.Hspec + +spec :: Pqi.Adapter -> SpecWith ByteString +spec adapter = + describe "execParams" do + describe "interrupted" do + it "the next calls return their own results" \conninfo -> + differential adapter conninfo scenario + +scenario :: Pqi.Connection -> IO (Maybe ResultObservation, Maybe ResultObservation, Pqi.TransactionStatus) +scenario connection = do + _ <- timeout 200_000 (Pqi.execParams connection "select 'slow' from pg_sleep (1)" [] Pqi.Text) + one <- observed "select 'one'" [] Pqi.Text connection + two <- observed "select 'two'" [] Pqi.Text connection + transactionStatus <- Pqi.transactionStatus connection + pure (one, two, transactionStatus) diff --git a/src/library/Pqi/Conformance/Operation/Getvalue.hs b/src/library/Pqi/Conformance/Operation/Getvalue.hs --- a/src/library/Pqi/Conformance/Operation/Getvalue.hs +++ b/src/library/Pqi/Conformance/Operation/Getvalue.hs @@ -7,11 +7,14 @@ import qualified Pqi import Pqi.Conformance.Harness +import qualified Pqi.Conformance.Operation.Getvalue.ManyRows as ManyRows import Pqi.Conformance.Prelude import Test.Hspec spec :: Pqi.Adapter -> SpecWith ByteString -spec adapter = +spec adapter = do + ManyRows.spec adapter + describe "getvalue" do it "reads cells, nulls, and degrades out of range" \conninfo -> differential adapter conninfo \connection -> do diff --git a/src/library/Pqi/Conformance/Operation/Getvalue/ManyRows.hs b/src/library/Pqi/Conformance/Operation/Getvalue/ManyRows.hs new file mode 100644 --- /dev/null +++ b/src/library/Pqi/Conformance/Operation/Getvalue/ManyRows.hs @@ -0,0 +1,75 @@ +-- | Coverage for reading every cell of a large result: 200,000 single-column +-- rows, fetched through 'Pqi.execParams' and scanned row by row with +-- 'Pqi.getisnull', 'Pqi.getlength' and 'Pqi.getvalue'. +-- +-- The divergence: @pqi-native@'s 'Pqi.Native.Types.mkResult' kept the rows +-- as a list and reached the requested row with @drop@ from its head on +-- every lookup, so scanning a result through the public accessors was +-- quadratic in its rows. @libpq@ indexes an array, so each lookup is +-- constant time. The values themselves came back intact, only slowly: the +-- issue measured 3.9 s to scan 40,000 rows against 17 ms with @libpq@. +-- +-- Where it surfaced: @pqi-native@ issue #15. +-- +-- Why this observation: the divergence is in cost, not in any returned +-- value, so the scan runs under a 'deadline' and the observation records +-- whether it finished. The reference scans these rows in about 0.1 s, and +-- the quadratic candidate would need over a minute, so the deadline sits +-- far from both. Every cell is checked as well, so that a fix cannot pass +-- by returning the wrong one. +module Pqi.Conformance.Operation.Getvalue.ManyRows + ( spec, + ) +where + +import qualified Data.ByteString as ByteString +import qualified Data.ByteString.Char8 as ByteString.Char8 +import qualified Pqi +import Pqi.Conformance.Harness +import Pqi.Conformance.Prelude +import System.Timeout (timeout) +import Test.Hspec + +rowCount :: Int32 +rowCount = 200_000 + +-- | How long the scan may take, in microseconds. +deadline :: Int +deadline = 5_000_000 + +spec :: Pqi.Adapter -> SpecWith ByteString +spec adapter = + describe "getvalue" do + describe "many rows" do + it "scans 200,000 rows within the deadline" \conninfo -> + differential adapter conninfo scenario + +-- | Fetch the rows, then report the result status, the row count, and +-- whether every cell checked out within the deadline (@Nothing@ when the +-- deadline passed). +scenario :: Pqi.Connection -> IO (Maybe (Pqi.ExecStatus, Int32, Maybe Bool)) +scenario connection = do + result <- + Pqi.execParams + connection + "select i :: int4 from generate_series (1, $1 :: int4) as i" + [Just (23, ByteString.Char8.pack (show rowCount), Pqi.Text)] + Pqi.Binary + for result \r -> do + status <- Pqi.resultStatus r + count <- Pqi.ntuples r + scanned <- timeout deadline (scan r count) + pure (status, count, scanned) + where + scan r count = go 0 + where + go row + | row >= count = pure True + | otherwise = do + isNull <- Pqi.getisnull r row 0 + size <- Pqi.getlength r row 0 + value <- Pqi.getvalue r row 0 + let decoded = fmap (ByteString.foldl' (\n byte -> n * 256 + fromIntegral byte) 0) value + if not isNull && size == 4 && decoded == Just (row + 1) + then go (row + 1) + else pure False