Skip to content

"WHERE" and "AND" clauses can produce syntax errors #172

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

Closed
rodionovv opened this issue Oct 13, 2024 · 3 comments · Fixed by #175
Closed

"WHERE" and "AND" clauses can produce syntax errors #172

rodionovv opened this issue Oct 13, 2024 · 3 comments · Fixed by #175

Comments

@rodionovv
Copy link

Hello!

First of all, thanks a lot for this amazing tool. I've been using it a lot in my personal projects.

Recently, I came across several issues when working with WHERE and AND clauses. Previously in #149 and v1.30.1 you've added validation of arrays used in these queries. But they can still produce empty clauses if array is not empty but contains only empty strings.

Example:

sb.Distinct().Select("*").Where("")
sb.And("", "")
sb.Distinct().Select("*").Where(sb.And("", ""))

will produce accordingly:

SELECT DISTINCT * WHERE
( AND )
SELECT DISTINCT * WHERE ( AND )

In v1.30.1 you've implemented counting size in bytes of strings to be inserted into AND condition. I think this might be handy in resolving this issue. If nothing can be added except for predefined constants, so i think nothing should be added. So the And function can be changed like this:

func (c *Cond) And(andExpr ...string) string {
	if len(andExpr) == 0 {
		return ""
	}

	exprByteLen := estimateStringsBytes(andExpr)
	if exprByteLen == 0 {
		return ""
	}

	buf := newStringBuilder()

	// Ensure that there is only 1 memory allocation.
	size := len(lparen) + len(rparen) + (len(andExpr)-1)*len(opAND) + exprByteLen
	buf.Grow(size)

	buf.WriteString(lparen)
	buf.WriteStrings(andExpr, opAND)
	buf.WriteString(rparen)
	return buf.String()
}

Same logic can be applied to WHERE clause, if all andExprs are empty, so don't generate them. So AddWhereExpr can be changed like this:

func (wc *WhereClause) AddWhereExpr(args *Args, andExpr ...string) *WhereClause {
	if len(andExpr) == 0 {
		return wc
	}

	andExprsBytesLen := 0
	for _, expr := range andExpr {
		andExprsBytesLen += len(expr)
	}

	if andExprsBytesLen == 0 {
		return wc
	}

	// Merge with last clause if possible.
	if len(wc.clauses) > 0 {
		lastClause := &wc.clauses[len(wc.clauses)-1]

		if lastClause.args == args {
			lastClause.andExprs = append(lastClause.andExprs, andExpr...)
			return wc
		}
	}

	wc.clauses = append(wc.clauses, clause{
		args:     args,
		andExprs: andExpr,
	})
	return wc
}

With these fixes examples provided previously will produce these values:

SELECT DISTINCT *
empty_string
SELECT DISTINCT *

If you think that these changes are legit and I'm not missing something, I can work on them in a PR. Examples above are just some raw ideas not the final code.

@huandu
Copy link
Owner

huandu commented Oct 15, 2024

Sounds reasonable. Besides And and AddWhereExpr, Not and Or can be changed in this way too.

@rodionovv
Copy link
Author

Yes, sure. I can work on it in a PR in a couples of day.

@huandu
Copy link
Owner

huandu commented Oct 17, 2024

It's highly appreciated! Thanks for your time.

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

Successfully merging a pull request may close this issue.

2 participants