Merge pull request #19714 from swordqiu/hotfix/qj-tag-filter-with-no-…

…value-revisit

fix: tag filter with no value revisit

pkg/cloudcommon/db/metadataresource.go

@@ -21,6 +21,7 @@ import (
 	"yunion.io/x/jsonutils"
 	"yunion.io/x/log"
 	"yunion.io/x/pkg/util/rbacscope"
+	"yunion.io/x/pkg/utils"
 	"yunion.io/x/sqlchemy"
 
 	"yunion.io/x/onecloud/pkg/apis"
@@ -58,23 +59,30 @@ func ObjectIdQueryWithPolicyResult(ctx context.Context, q *sqlchemy.SQuery, mana
 }
 
 func ObjectIdQueryWithTagFilters(ctx context.Context, q *sqlchemy.SQuery, idField string, modelName string, filters tagutils.STagFilters) *sqlchemy.SQuery {
-	if len(filters.Filters) > 0 {
-		sq := objIdQueryWithTags(ctx, modelName, filters.Filters)
-		if sq != nil {
-			sqq := sq.SubQuery()
-			q = q.Join(sqq, sqlchemy.Equals(q.Field(idField), sqq.Field("obj_id")))
+	if len(filters.Filters) > 0 || len(filters.NoFilters) > 0 {
+		idSubQ := q.Copy().SubQuery().Query()
+		idSubQ.AppendField(sqlchemy.DISTINCT(idField, idSubQ.Field(idField)))
+		subQ := idSubQ.SubQuery()
+		if len(filters.Filters) > 0 {
+			sq := objIdQueryWithTags(ctx, subQ, idField, modelName, filters.Filters)
+			if sq != nil {
+				sqq := sq.SubQuery()
+				q = q.Join(sqq, sqlchemy.Equals(q.Field(idField), sqq.Field(idField)))
+			}
 		}
-	}
-	if len(filters.NoFilters) > 0 {
-		sq := objIdQueryWithTags(ctx, modelName, filters.NoFilters)
-		if sq != nil {
-			q = q.Filter(sqlchemy.NotIn(q.Field(idField), sq.SubQuery()))
+		if len(filters.NoFilters) > 0 {
+			sq := objIdQueryWithTags(ctx, subQ, idField, modelName, filters.NoFilters)
+			if sq != nil {
+				sqq := sq.SubQuery()
+				q = q.LeftJoin(sqq, sqlchemy.Equals(q.Field(idField), sqq.Field(idField)))
+				q = q.Filter(sqlchemy.IsNull(sqq.Field(idField)))
+			}
 		}
 	}
 	return q
 }
 
-func objIdQueryWithTags(ctx context.Context, modelName string, tagsList []map[string][]string) *sqlchemy.SQuery {
+func objIdQueryWithTags(ctx context.Context, objIdSubQ *sqlchemy.SSubQuery, idField string, modelName string, tagsList []map[string][]string) *sqlchemy.SQuery {
 	manager := GetMetadaManagerInContext(ctx)
 	metadataResQ := manager.Query().Equals("obj_type", modelName).SubQuery()
 
@@ -83,29 +91,37 @@ func objIdQueryWithTags(ctx context.Context, modelName string, tagsList []map[st
 		if len(tags) == 0 {
 			continue
 		}
-		metadataView := metadataResQ.Query(metadataResQ.Field("obj_id").Label("obj_id"))
+		objIdQ := objIdSubQ.Query()
+		objIdQ = objIdQ.AppendField(objIdQ.Field(idField))
 		for key, val := range tags {
-			q := metadataResQ.Query(metadataResQ.Field("id"))
-			q = q.Equals("key", key)
+			sq := metadataResQ.Query().Equals("key", key).SubQuery()
+			objIdQ = objIdQ.LeftJoin(sq, sqlchemy.Equals(objIdQ.Field(idField), sq.Field("obj_id")))
 			if len(val) > 0 {
-				q = q.In("value", val)
+				if utils.IsInArray(tagutils.NoValue, val) {
+					objIdQ = objIdQ.Filter(sqlchemy.OR(
+						sqlchemy.In(sq.Field("value"), val),
+						sqlchemy.IsNull(sq.Field("value")),
+					))
+				} else {
+					objIdQ = objIdQ.Filter(sqlchemy.In(sq.Field("value"), val))
+				}
+			} else {
+				objIdQ = objIdQ.Filter(sqlchemy.IsNotNull(sq.Field("obj_id")))
 			}
-			sq := q.SubQuery()
-			metadataView = metadataView.Join(sq, sqlchemy.Equals(metadataView.Field("id"), sq.Field("id")))
 		}
-		queries = append(queries, metadataView.Distinct())
+		queries = append(queries, objIdQ.Distinct())
 	}
 	if len(queries) == 0 {
 		return nil
 	}
-	var query sqlchemy.IQuery
+	var query *sqlchemy.SQuery
 	if len(queries) == 1 {
-		query = queries[0]
+		query = queries[0].(*sqlchemy.SQuery)
 	} else {
 		uq, _ := sqlchemy.UnionWithError(queries...)
 		query = uq.Query()
 	}
-	return query.SubQuery().Query()
+	return query
 }
 
 func (meta *SMetadataResourceBaseModelManager) ListItemFilter(

pkg/util/tagutils/filters.go

@@ -30,48 +30,12 @@ func splitValues(values []string) (bool, []string) {
 	}
 }
 
-func (ts TTagSet) toFilters() (map[string][]string, map[string][]string) {
-	filter := tagset2Map(ts)
-	pos := make(map[string][]string)
-	neg := make(map[string][]string)
-	negExist := false
-	for k, v := range filter {
-		noval, values := splitValues(v)
-		if noval {
-			negExist = true
-			if len(values) > 0 {
-				pos[k] = values
-			}
-			neg[k] = []string{}
-		} else {
-			pos[k] = values
-			neg[k] = values
-		}
-	}
-	if !negExist {
-		neg = nil
-	}
-	return pos, neg
-}
-
 func (tf *STagFilters) AddFilter(ts TTagSet) {
-	pos, neg := ts.toFilters()
-	if pos != nil {
-		tf.Filters = append(tf.Filters, pos)
-	}
-	if neg != nil {
-		tf.NoFilters = append(tf.NoFilters, neg)
-	}
+	tf.Filters = append(tf.Filters, tagset2Map(ts))
 }
 
 func (tf *STagFilters) AddNoFilter(ts TTagSet) {
-	pos, neg := ts.toFilters()
-	if pos != nil {
-		tf.NoFilters = append(tf.NoFilters, pos)
-	}
-	if neg != nil {
-		tf.Filters = append(tf.Filters, neg)
-	}
+	tf.NoFilters = append(tf.NoFilters, tagset2Map(ts))
 }
 
 func (tf *STagFilters) AddFilters(tsl TTagSetList) {