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

#2794 is bug #2795

Open
it512 opened this issue Sep 15, 2023 · 8 comments
Open

#2794 is bug #2795

it512 opened this issue Sep 15, 2023 · 8 comments

Comments

@it512
Copy link
Contributor

it512 commented Sep 15, 2023

#2794
no ... I think this is bug....

uuid is value type , like int.
default value is zero value ( uuid.Nil), in GraphQL is UUID!
nil ('null") is *uuid.UUID in Graphql is UUID. ( no !)

so this is bug!

@StevenACoffman @0x221A

Originally posted by @it512 in #2794 (comment)

@it512
Copy link
Contributor Author

it512 commented Sep 15, 2023

@StevenACoffman

@it512
Copy link
Contributor Author

it512 commented Sep 15, 2023

type Todo {                                                                                                           
      id: ID!                                                                                                             
      text: String!                                                                                                       
       done: Boolean!                                                                                                      
       uid: UUID!                                                                                                          
       puid:UUID                                                                                                           
  }                                                                                                                     
 
type Query {                                                                                                          
   todos: [Todo!]!                                                                                                     
}       

{
  todos{
    uid
    puid
  }
}

go code

func (r *queryResolver) Todos(ctx context.Context) ([]*model.Todo, error) {                                           
   return []*model.Todo{                                                                                             
      {ID: "1", Text: "hello", Done: true, UID: uuid.New(), Puid: &uuid.UUID{}},                                    
     {ID: "2", Text: "world", Done: false, UID: uuid.New(), Puid: nil},                                            
    }, nil                                                                                                            
}                                                                                                                     

#2794 return

{
  "data": {
    "todos": [
      {
        "uid": "2156c78b-fa83-4ee7-ab24-5a871a433e85",
        "puid": null ---------------  &uuid.UUID{} is zero value (00000000-0000-0000-0000-000000000000) *** bug!!! ***
      },
      {
        "uid": "ff8b4cf9-db3a-4634-842f-a9394afa590d",
        "puid": null
      }
  }

ok result (v v0.17.37 )

{
  "data": {
    "todos": [
      {
        "uid": "0171dcab-6c95-4d1f-ac4a-a6a0f72d869b",
        "puid": "00000000-0000-0000-0000-000000000000" ---- zero value ok !!!
      },
      {
        "uid": "437e9668-ff9e-421d-b7d4-323dfbe2e244",
        "puid": null -------------- *uuid.UUID set to nil return null is ok (v0.17.37  is ok!!!)
      }
    ]
  }
}

@it512
Copy link
Contributor Author

it512 commented Sep 15, 2023

if need I can commit PR to fix #2794
@StevenACoffman

@it512 it512 changed the title #2794 is bug? #2794 is bug Sep 15, 2023
@StevenACoffman
Copy link
Collaborator

@0x221A Can you please comment on this?

@0x221A
Copy link
Contributor

0x221A commented Sep 16, 2023

@StevenACoffman For me the user wants to return a zero value uuid instead of null for some reason then I will suggest using the custom mapping. Returning null first is the best way unless the user knows what they're dealing with. What u think?

@0x221A
Copy link
Contributor

0x221A commented Sep 16, 2023

Using the pointer of uuid is the other approach but I'm concerned when using it with SQL scan interface or an unexpected nil address panic.

@it512
Copy link
Contributor Author

it512 commented Sep 16, 2023

show your code, let me see see.

@it512
Copy link
Contributor Author

it512 commented Sep 16, 2023

@0x221A look this
https://github.com/google/uuid/blob/v1.3.1/sql.go#L15

func (uuid *UUID) Scan(src interface{}) error {
	switch src := src.(type) {
	case nil:
		return nil // look this

	case string:
		// if an empty UUID comes from a table, we return a null UUID
		if src == "" {
			return nil
		}

		// see Parse for required string format
		u, err := Parse(src)
		if err != nil {
			return fmt.Errorf("Scan: %v", err)
		}

		*uuid = u

	case []byte:
		// if an empty UUID comes from a table, we return a null UUID
		if len(src) == 0 {
			return nil
		}

		// assumes a simple slice of bytes if 16 bytes
		// otherwise attempts to parse
		if len(src) != 16 {
			return uuid.Scan(string(src))
		}
		copy((*uuid)[:], src)

	default:
		return fmt.Errorf("Scan: unable to scan type %T into UUID", src)
	}

	return nil
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants