Support sql.ErrNoRows

[xo]

Support sql.ErrNoRows

[kenshaw]

I understand the rationale, but I'm not sure if I agree if this is the logical behavior, as I don't think this would fit for all use cases. Part of what makes xo useful is that you're able to bring your own templates into the mix, and I think that this would be a perfect example of why you'd want to use custom templates.

As a side note, please note that you're basically dropping an error condition (or not reacting to it) and converting a specific error to nil. I don't think one could consider this idiomatic Go.

[xeoncross]

Excuse my laziness, but I'm just getting over a fever.

Edit https://github.com/knq/xo/blob/master/internal/argtype.go

// HandleNoRow and and return nil instead of an error for it
HandleNoRow bool `arg:"--handle-norow,help:return nil instead of an error if no record is found"`

And then in https://github.com/knq/xo/blob/master/templates/postgres.query.go.tpl#L19

if err != nil {
	{{- if .HandleNoRow }}
	if err == sql.ErrNoRows {
		return nil, nil
	}
	{{ end }}
	return nil, err
}

[xeoncross]

Thanks for the quick reply. Let me address your points:

I understand the rationale, but I'm not sure if I agree if this is the logical behavior, as I don't think this would fit for all use cases.

This wouldn't be the default behavior - but a highly useful small feature.

Part of what makes xo useful is that you're able to bring your own templates into the mix, and I think that this would be a perfect example of why you'd want to use custom templates.

It seems overkill to write your own templates just for this.

As a side note, please note that you're basically dropping an error condition (or not reacting to it) and converting a specific error to nil. I don't think one could consider this idiomatic Go.

It's not really an error condition. It's a marker you are supposed to check. No results is not treated as an error in any language but Go... which is why the recommend you check that everything is ok by using sql.ErrNoRows.

No information is being lost but the marker for "empty" is simply a nil result instead of an nil result wrapped up as an Error.

By allowing the check to be performed in the Model we can keep the code more DRY.

[kenshaw]

Let me think about it for a bit, and I'll make a decision. Already xo has "too many" options, and also this would need to be applied to more than one template. Again, I'm still thinking the appropriate / correct approach is to simply use your own templates. "xo is not a silver bullet"

[xeoncross]

I understand, make sure your convinced this is a good feature (After several Go apps I am pretty convinced it is). Here is one more very common example of dealing with a user struct in different parts of the application.

user, err = models.GetUserByID/Email/Username(db, param)
if err != nil {
	if err == sql.ErrNoRows {
		user = nil
	} else {
		log.Fatal(err)
	}
}

vs

user, err = models.GetUserByID/Email/Username(db, param)
if err != nil {
	log.Fatal(err)
}

They both continue with the following user handling

if user == nil {
  // deny, login, redirect, 404
} else {
  
}

To me, keeping database-specific knowledge out of the controller also seem cleaner. Every file I use a user object on shouldn't have to know (or import) the special sql constants to use the results.

[turnkey-commerce]

I tend to agree that no rows returned from a specific query is not so much an exceptional error case and may occur often in cases where custom user criteria are allowed. The case of returning nil allows the application to react to that case more naturally and separately handle a hard error such as the database not being available.

At the very least we should return a specific error type for the NoRows case if it is still going to return an error.

[xeoncross]

At the very least we should return a specific error type for the NoRows case if it is still going to return an error.

Returning an error marking an error marking a nil isn't exactly a step up. Then again, if your PHP/Java it's one more abstraction, so I guess that's a win.

[turnkey-commerce]

I see your point and am definitely not one of those :)