Skip to content
This repository has been archived by the owner on Sep 30, 2024. It is now read-only.

bug in the third package sqlutils #943

Closed
yangeagle opened this issue Jul 27, 2019 · 1 comment · Fixed by #946
Closed

bug in the third package sqlutils #943

yangeagle opened this issue Jul 27, 2019 · 1 comment · Fixed by #946

Comments

@yangeagle
Copy link
Contributor

I am not sure whether this question is suitable here.

I find a problem in the third package github.com/openark/golib/sqlutils used in orchestrator.

package: github.com/openark/golib/sqlutils
file: golib/sqlutils/sqlutils.go
code:

// QueryRowsMap is a convenience function allowing querying a result set while poviding a callback
// function activated per read row.
func QueryRowsMap(db *sql.DB, query string, on_row func(RowMap) error, args ...interface{}) error {
	var err error
	defer func() {
		if derr := recover(); derr != nil {
			err = errors.New(fmt.Sprintf("QueryRowsMap unexpected error: %+v", derr))
		}
	}()

... ...
	return err
}

The err modified in defer will not return to the caller.

For example:

package main

import (
        "fmt"
)

func main(){
        i := test()
        fmt.Println("main i:", i)
}

func test() int {
        var a int
        defer func () {
                a = 2
        }()

        a = 1
        return a
}

output:
main i: 1

The correct usage like this:

package main

import (
        "fmt"
)

func main(){
        i := test()
        fmt.Println("main i:", i)
}

func test() (a int) {
        defer func () {
                a = 2
        }()

        a = 1
        return a
}

output:
main i: 2

I also submitted an issue here.

Please check it.
cc @MOON-CLJ

@shlomi-noach
Copy link
Collaborator

Thank you. Please consider #946 as a fix.

zmoazeni added a commit to zmoazeni/gh-ost that referenced this issue Aug 5, 2019
This is copied from openark/orchestrator#946 with
a slight tweak to the an indent.

Originates from the PR openark/orchestrator#943
that applies to this codebase too.

When we define the variable and attempt to set it in a defer function,
the value we're returning isn't what we expect.

Instead we can name the return values as variables and set that instead.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants
@shlomi-noach @yangeagle and others