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

support fields linked with ops #135

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Guojunzhou-git
Copy link

@Guojunzhou-git Guojunzhou-git commented Feb 13, 2023

map with key foo> instead of foo > got field="foo>" operator="=", then where condition is foo>=.

builder.BuildSelect(map[string]interface{}{
    "foo>": "bar"
})

got

SELECT * FROM tb WHERE (foo>=?)"

when no space in key, not default field=key and operator="=", but try to parse fileds linked with ops, such as foo>,foo>=,foo<,foo<=,foo<>,foo!=,foo=

@caibirdme
Copy link
Contributor

可以讨论下必要性,我个人觉得这个没多大必要:

  1. 大多数时候用的都是等于比较,即 "a": 1。如果合入这个pr之后,所有的等于比较都要增加额外的负担来检查key里是否包涵比较符号,对性能有一定影响
  2. 用户只要知道中间要打个空格,是不是以后就完全用不到这个feature了

@Guojunzhou-git
Copy link
Author

可以讨论下必要性,我个人觉得这个没多大必要:

  1. 大多数时候用的都是等于比较,即 "a": 1。如果合入这个pr之后,所有的等于比较都要增加额外的负担来检查key里是否包涵比较符号,对性能有一定影响
  2. 用户只要知道中间要打个空格,是不是以后就完全用不到这个feature了

个人建议哈:

  1. 比起 a>= 解析为 a>== 的 sql 错误来讲,b>解析为 b>= 的表现会比较难以发现。引入此 pr 后,使用起来会更友好些。
  2. opCanLinkedWithField涉及此问题的运算符只有7个:4个2字符运算符(将导致sql错误)和3个1字符运算符(将导致语义上的变化),对性能的影响不是很大。

综上,还是建议能更多的考虑易用和友好

@Guojunzhou-git
Copy link
Author

func splitKey(key string, val interface{}) (field string, operator string, err error) {
	key = strings.Trim(key, " ")
	if "" == key {
		err = errSplitEmptyKey
		return
	}
	idx := strings.IndexByte(key, ' ')
	if idx == -1 {
		field = key
		operator = "="
		if isFieldContainOp(field) {
			field, operator = splitKeyNoSpace(field)
		}
		if reflect.ValueOf(val).Kind() == reflect.Slice {
			operator = "in"
		}
	} else {
		field = key[:idx]
		operator = strings.Trim(key[idx+1:], " ")
		operator = removeInnerSpace(operator)
	}
	return
}

func isFieldContainOp(field string) (contains bool) {
	if field == "" {
		return
	}
	ecof := field[len(field)-1:]
	contains = ecof == opEq || ecof == opLt || ecof == opGt
	return
}
`

@Guojunzhou-git
Copy link
Author

可以尝试 isFieldContainOp 判定下是否需要

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