Re: [Xen-devel] [OSSTEST PATCH 5/7] Database locking: Tcl: for errorCode, use pg_exec, not pg_execute

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

On Thu, 2016-01-07 at 19:38 +0000, Ian Jackson wrote:
> We would like to be able to retry db transactions.ÂÂTo do this we need
> to know why they failed (if they did).
> 
> But pg_execute does not set errorCode.ÂÂ(This is clearly a bug.)ÂÂAnd
> since it immediately discards a failed statement, any error
> information has been lost by the time pg_execute returns.
> 
> So, instead, use pg_exec, and manually mess about with fishing
> suitable information out of a failed statement handle, and generating
> an appropriate errorCode.
> 
> There are no current consumers of this errorCode: that will come in a
> moment.
> 
> A wrinkle is that as a result it is no longer possible to use
> db-execute on a SELECT statement nor db-execute-array on a non-SELECT
> statement.ÂÂThis is because the 1. the `ok' status that we have to

Stray "the" before "1.".

> check for is different for statements which are commands and ones
> which return tupes and 2. we need to fish a different return value out

"tuples"

> of the statement handle (-cmdTuples vs -numTuples).ÂÂBut all uses in
> the codebase are now fine for this distinction.

Does this imply that db-execute-array could be renamed db-execute-select,
or even just db-select?

> 
> Signed-off-by: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
> ---
> Âtcl/JobDB-Executive.tcl |ÂÂÂ54
> ++++++++++++++++++++++++++++++++++++++++++++---
> Â1 file changed, 51 insertions(+), 3 deletions(-)
> 
> diff --git a/tcl/JobDB-Executive.tcl b/tcl/JobDB-Executive.tcl
> index bbce6fc..ed9abbb 100644
> --- a/tcl/JobDB-Executive.tcl
> +++ b/tcl/JobDB-Executive.tcl
> @@ -121,13 +121,61 @@ proc db-execute-debug {stmt} {
> Â     puts stderr "EXECUTING >$stmt<"
> ÂÂÂÂÂ}
> Â}
> +
> +proc db--exec-check {shvar stmt expected_status body} {
> +ÂÂÂÂ# pg_execute does not set errorCode and it throws away the
> +ÂÂÂÂ# statement handle so we can't get the error out.ÂÂSo
> +ÂÂÂÂ# use pg_exec, as wrapped up here.
> +
> +ÂÂÂÂ# db--exec-check executes stmt and checks that the status is
> +ÂÂÂÂ# `expected_status'.ÂÂIf OK, executes body with $shvar set to the
> +ÂÂÂÂ# stmt handle.ÂÂÂOtherwise throws with errorCode
> +ÂÂÂÂ#ÂÂÂ{OSSTEST-PSQL <pg-status> <pg-sqlstate>}
> +
> +ÂÂÂÂglobal errorInfo errorCode
> +ÂÂÂÂupvar 1 $shvar sh
> +
> +ÂÂÂÂset sh [pg_exec dbh $stmt]
> +
> +ÂÂÂÂset rc [catch {
> +     set status [pg_result $sh -status]
> +     if {[string compare $status $expected_status]} {
> +     ÂÂÂÂset emsg [pg_result $sh -error]
> +     ÂÂÂÂset sqlstate [pg_result $sh -error sqlstate]
> +     ÂÂÂÂif {![string length $emsg]} {
> +             set emsg "osstest expected status $expected_status got
> $status"
> +     ÂÂÂÂ}
> +     ÂÂÂÂset context [pg_result $sh -error context]
> +     ÂÂÂÂerror $emsg \
> +             "ÂÂÂÂwhile executing SQL\n$stmt\nÂÂÂÂin SQL
> context\n$context" \
> +             [list OSSTEST-PSQL $status $sqlstate]
> +     }
> +     uplevel 1 $body
> +ÂÂÂÂ} emsg]
> +
> +ÂÂÂÂset ei $errorInfo
> +ÂÂÂÂset ec $errorCode
> +ÂÂÂÂcatch { pg_result $sh -clear }
> +
> +ÂÂÂÂreturn -code $rc -errorinfo $ei -errorcode $ec $emsg
> +}
> +
> Âproc db-execute {stmt} {
> ÂÂÂÂÂdb-execute-debug $stmt
> -ÂÂÂÂuplevel 1 [list pg_execute dbh $stmt]
> +ÂÂÂÂdb--exec-check sh $stmt PGRES_COMMAND_OK {
> +     return [pg_result $sh -cmdTuples]
> +ÂÂÂÂ}
> Â}
> -proc db-execute-array {arrayvar stmt args} {
> +proc db-execute-array {arrayvar stmt {body {}}} {
> ÂÂÂÂÂdb-execute-debug $stmt
> -ÂÂÂÂuplevel 1 [list pg_execute -array $arrayvar dbh $stmt] $args
> +ÂÂÂÂdb--exec-check sh $stmt PGRES_TUPLES_OK {
> +     set nrows [pg_result $sh -numTuples]
> +     for {set row 0} {$row < $nrows} {incr row} {
> +     ÂÂÂÂuplevel 1 [list pg_result $sh -tupleArray $row $arrayvar]
> +     ÂÂÂÂuplevel 1 $body
> +     }
> +     return $nrows
> +ÂÂÂÂ}
> Â}
> Â
> Âproc lock-tables {tables} {

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel