Skip to content

Set operators having a private return type is awkward sometimes #485

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

Open
kblomster opened this issue Apr 10, 2025 · 1 comment
Open

Set operators having a private return type is awkward sometimes #485

kblomster opened this issue Apr 10, 2025 · 1 comment
Labels

Comments

@kblomster
Copy link
Contributor

kblomster commented Apr 10, 2025

The pain point

In our code base, when a select statement gets used in more than one place, we usually create it with a function, like

func selectSomeComplexThing() postgres.SelectStatement {
	return table.SomeTable.SELECT(...)
}

This works great, but if you start adding set operators like UNION/EXCEPT/INTERSECT to this statement, then the return type can't be SelectStatement anymore. The actual return type of these operators is setStatement, which is not exported. We can use Statement, which works, but then the resulting statement object doesn't have ORDER_BY, OFFSET and LIMIT anymore, so the caller can't add those (and that is a pattern we use often).

In many cases this can be worked around by wrapping the statement with .AsTable("alias").SELECT(...), but this introduces an additional table alias in the generated SQL, which can mess with the projection aliasing and result mapping. This workaround isn't always feasible, either. For example, if you're trying to use the statement in a recursive CTE, the UNION [ALL|DISTINCT] needs to be at the top level or Postgres throws an error ([42P19] ERROR: recursive query "cte_name_here" does not have the form non-recursive-term UNION [ALL] recursive-term). So, in that case the only solution is to pass the return value from UNION directly to CommonTableExpression.AS() without assigning it an explicit type, which works but places some limitations on how the query building can be structured.

This is certainly not a huge problem, but it feels kind of clunky and people who aren't intimately familiar with Jet tend to get confused by it.

Side note

CommonTableExpression.AS() accepts the type jet.SerializerHasProjections. SelectTable is compatible with that type, but if you actually try that, something like this

import (
	"fmt"
	jet "github.com/go-jet/jet/v2/postgres"
)

func selectRecursiveStuff() jet.SelectTable {
	return jet.
		UNION_ALL(
			jet.SELECT(jet.String("foo")),
			jet.SELECT(jet.String("bar"))).
		AsTable("foo")
}

func selectThings() {
	cte := jet.CTE("things").AS(selectRecursiveStuff())
	stmt := jet.WITH_RECURSIVE(cte)(
		jet.SELECT(jet.STAR).
			FROM(cte))
	fmt.Println(stmt.DebugSql())
}

then the generated SQL you get is

WITH RECURSIVE things AS (
     (
          SELECT 'foo'::text
     )
     UNION ALL
     (
          SELECT 'bar'::text
     )
) AS foo
SELECT *
FROM things;

which is invalid (WITH things AS (...) AS foo). Which makes sense really, you're trying to use a table reference where a query is expected, but I'd expect the type system to catch that.

Oddly enough, CommonTableExpression.AS_NOT_MATERIALIZED() accepts the type jet.SerializerStatement instead, which prevents this error. Is it just an API backwards compatibility issue that keeps .AS() the way it is?

Describe the solution you'd like

I understand the desire to not make the public API maintenance burden bigger than what is strictly necessary, but exporting setStatement would be a nice convenience for me.

I do acknowledge that this is a pretty niche use case, though.

@go-jet
Copy link
Owner

go-jet commented Apr 12, 2025

I agree with making setStatement public.

Regarding the use of a subquery (or SelectTable) as a CTE, this appears to be an unintentional side effect of adding support for VALUES statements, which are also of type SelectTable.

AS_NOT_MATERIALIZED() accepting the type jet.SerializerStatement is probably a bug, because VALUES can't be passed as this type.

@go-jet go-jet added the good first issue Good for newcomers label Apr 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants