Skip to content

x/sqlbuilder: Implement MultiColumnIn - #45

Merged
AlexisMontagne merged 1 commit into
masterfrom
am/multi-clause-in
Mar 26, 2026
Merged

x/sqlbuilder: Implement MultiColumnIn#45
AlexisMontagne merged 1 commit into
masterfrom
am/multi-clause-in

Conversation

@AlexisMontagne

@AlexisMontagneAlexisMontagne commented Mar 25, 2026

Copy link
Copy Markdown
Member

What does this PR do?

In our internal code base we have a lot of queries that look like:

SELECT bar FROM foo WHERE (a = $1 AND b = $2) OR (a = $3 AND b = $4)

For our current version of postgres it suffers from pretty poor performances.

This new predicate clause, should make it easy to write query like:

SELECT bar FROM foo WHERE (a, b) IN (($1, $2), ($3, $4))

What are the observable changes?

Good PR checklist

  • Title makes sense
  • Is against the correct branch
  • Only addresses one issue
  • Properly assigned
  • Added/updated tests
  • Added/updated documentation
  • Properly labeled

Additional Notes

@AlexisMontagneAlexisMontagne self-assigned this Mar 25, 2026
@AlexisMontagne
AlexisMontagne requested a review from a team as a code ownerMarch 25, 2026 00:32
@AlexisMontagne
AlexisMontagne requested review from FlorianRichardUPF and xgoffin and removed request for a teamMarch 25, 2026 00:32

func (mc *multiColumnInClause) WriteTo(w QueryWriter, vs map[string]interface{}) error {
if _, ok := vs[mc.k]; !ok || len(mc.ms) == 0 {
io.WriteString(w, "1=0")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)

return ErrMissingKey{b}
}

io.WriteString(w, w.RedeemVariable(vv.Interface()))

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)

io.WriteString(w, w.RedeemVariable(vv.Interface()))

if j < len(bindings)-1 {
io.WriteString(w, ", ")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)

}

if v.Len() == 0 {
io.WriteString(w, "1=0")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)

}

if i < v.Len()-1 {
io.WriteString(w, ", ")

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
Error return value of io.WriteString is not checked (errcheck)

func (mc *multiColumnInClause) WriteTo(w QueryWriter, vs map[string]interface{}) error {
if _, ok := vs[mc.k]; !ok || len(mc.ms) == 0 {
io.WriteString(w, "1=0")
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
return with no blank line before (nlreturn)

}

fmt.Fprint(w, ")")
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
return with no blank line before (nlreturn)


if v.Len() == 0 {
io.WriteString(w, "1=0")
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
return with no blank line before (nlreturn)

}

io.WriteString(w, ")")
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
return with no blank line before (nlreturn)

func writeInClause(w QueryWriter, vv interface{}, k string) error {
return writeInValues(w, vv, k, func(w QueryWriter, elem interface{}) error {
io.WriteString(w, w.RedeemVariable(elem))
return nil

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚫 [golangci]reported by reviewdog 🐶
return with no blank line before (nlreturn)

@tgallicetgallice left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +329 to +330
stmt: "SELECT bar FROM foo WHERE (a, b) IN (($1, $2), ($3, $4))",
args: []interface{}{1, 2, 3, 4},

@xgoffinxgoffinMar 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@AlexisMontagne this bothers me: don't we want a in (1, 3) and b in (2, 4)?

As I read this statement it does a in (1, 2) and b in (3, 4). Am I confused? 👀

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A IN (1, 3) AND B IN (2, 4) would access the tuple: a = 1, b = 4 and also a=3, b = 2 neither of those should be included

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OOOOOOOOOOOOK looks like I can't read sql 💀 now that I see it again I get it, jesus, that was complicated

@xgoffinxgoffin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modernize interface{} pls 🙏

SelectClauses: []Marker{Column("bar")},
WhereClause: StaticMultiColumnIn([]Marker{Column("a"), Column("b")}, []map[string]interface{}{{"a": 1, "b": 2}, {"a": 3, "b": 4}}),
},
stmt: "SELECT bar FROM foo WHERE (a, b) IN (($1, $2), ($3, $4))",

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, this seems swapped

@AlexisMontagne
AlexisMontagne merged commit 56c3a9d into masterMar 26, 2026
3 of 4 checks passed
@AlexisMontagne
AlexisMontagne deleted the am/multi-clause-in branch March 26, 2026 18:15
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@AlexisMontagne@tgallice@xgoffin@upfluence-bot