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

Fix extra 'AND' when len(values) == 0 ON IN.NegationBuild() #4618

Merged
merged 1 commit into from
Aug 20, 2021

Conversation

secake
Copy link
Contributor

@secake secake commented Aug 16, 2021

  • Do only one thing
  • Non breaking API changes
  • Tested

What did this pull request do?

Three reasons for this PR:

  1. When DB.Not(map[string]interface{}{"name": []string{}}).Where("age = ?", 10), Generate sql SELECT * FROM ``users`` WHERE AND ``age`` = 10; , it is a bad sql.
  2. In some database, like postgresql, when exec ``name`` NOT IN ('jinzhu'), the result dose not contain the row where name == NULL, so when is not in an empty array, to some extent, it can be considered that name IS NOT NULL.
  3. NegationBuild() should be the inverse of Build(), but when len(values)==0, the Build() return the row where name==NULL, so NegationBuild() should not return the row where name==NULL.

User Case Description

DB.Not(map[string]interface{}{"name": []string{}}).Where("age = ?", 10).Find(&User{})`   

Thanks!

@secake
Copy link
Contributor Author

secake commented Aug 17, 2021

Suggestion

In other ORM, like django, when use User.objects.filter(name__in=[]), the result dose not contain the row where name==NULL, when use User.objects.excludes(name__in=[]), the result return all rows, also contains the row where name==NULL, as follows:
select id,name from users;

id name
1 jinzhu
2 NULL
User.objects.filter(name__in=[]).count()  // 0
User.objects.exclude(name__in=[]).count()` // 2

So I suggest the following implementation,if you think this suggestion works, I can implement it :

func (in IN) Build(builder Builder) {
	builder.WriteQuoted(in.Column)

	switch len(in.Values) {
	case 0:
		builder.WriteString(" 1=2")
	case 1:
		if _, ok := in.Values[0].([]interface{}); !ok {
			builder.WriteString(" = ")
			builder.AddVar(builder, in.Values[0])
			break
		}

		fallthrough
	default:
		builder.WriteString(" IN (")
		builder.AddVar(builder, in.Values...)
		builder.WriteByte(')')
	}
}

func (in IN) NegationBuild(builder Builder) {
	builder.WriteQuoted(in.Column)
	switch len(in.Values) {
	case 0:
		builder.WriteString(" 1=1")
	case 1:
		if _, ok := in.Values[0].([]interface{}); !ok {
			builder.WriteString(" <> ")
			builder.AddVar(builder, in.Values[0])
			break
		}

		fallthrough
	default:
		builder.WriteString(" NOT IN (")
		builder.AddVar(builder, in.Values...)
		builder.WriteByte(')')
	}
}

@jinzhu jinzhu merged commit 093694f into go-gorm:master Aug 20, 2021
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

Successfully merging this pull request may close these issues.

2 participants