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.

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

Scanning results inside iteration (Result.Iterate, document.ScanIterator) is completely broken #497

Closed
renbou opened this issue Apr 26, 2023 · 7 comments · Fixed by #499
Closed
Labels
bug Something isn't working

Comments

@renbou
Copy link

renbou commented Apr 26, 2023

What version of Genji are you using?

Genji v0.15.1

What did you do?

Scanning documents during iteration (either inside Result.Iterate manually, or using document.ScanIterator) results in invalid, "broken" documents after the iteration is finished. That is, during iteration, the results are valid, however, after the scan is complete, the scanned items are completely broken. Well, at least the strings, that is, since it seems like they are directly converted from some underlying []byte slice, which is why their content changes after the scan.

Here's an example of the bug on the latest version:
https://go.dev/play/p/qRKyf_jRqlR

What did you expect to see?

Expected the value during iteration and after iteration to be equal to main.item{A:2, B:"sample text 2"}.

What did you see instead?

During iteration the value is, indeed, scanned correctly as main.item{A:2, B:"sample text 2"}. After iteration, however, its contents change to main.item{A:2, B:"sample text 1"}.

@renbou renbou added the bug Something isn't working label Apr 26, 2023
@fasionchan
Copy link

I have the same problem as well... I wasted a whole day on it since I can't imagine that this method would have such a serious yet obvious BUG!!!!!

Any way, I have found a tricky way to solve it, but not perfect: marshal to json first, and then unmarshal back to my struct.

        if err := result.Iterate(func(doc types.Document) error {
		bytes, err := doc.MarshalJSON()
		if err != nil {
			return err
		}

		data := new(T)
		if err := json.Unmarshal(bytes, data); err != nil {
			return err
		}
		// fmt.Println("DataBefore", data)
		// if err := document.StructScan(doc, data); err != nil {
		// 	return nil
		// }

		fmt.Println("DataAfter", data)
		datas = append(datas, data)

		return nil
	}); err != nil {
		return nil, err
	}

@fasionchan
Copy link

However, I think scanValue should make an deepcopy from the underlying data, in order to solve the problem complete.

@fasionchan
Copy link

func StructScan(doc types.Document, data interface{}) error {
	bytes, err := doc.MarshalJSON()
	if err != nil {
		return err
	}

	return json.Unmarshal(bytes, data)
}

@asdine
Copy link
Collaborator

asdine commented May 6, 2023

This is indeed a bug in StructScan. The iteration itself works as expected and only copies data when needed, but StructScan should definitely create a deep copy.
I'll publish a fix this week.

@p4u
Copy link

p4u commented Jun 17, 2023

Is there a fix for this issue @asdine? thanks.

@asdine
Copy link
Collaborator

asdine commented Jun 18, 2023

#499 should have fixed the issue, I will release a patch soon

@asdine
Copy link
Collaborator

asdine commented Jun 18, 2023

I released v0.15.2, let me know if you are still experiencing the bug.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants