From a550a77e3bd269952dfea7b165d81b0f378c8e15 Mon Sep 17 00:00:00 2001 From: Richard Bizik Date: Sat, 19 Mar 2022 14:02:57 +0100 Subject: [PATCH 1/3] Fix nullable colums for nested selects in left and full joins --- internal/compiler/output_columns.go | 9 ++++ .../join_left/postgresql/go/query.sql.go | 45 +++++++++++++++++++ .../testdata/join_left/postgresql/query.sql | 11 +++++ internal/sql/ast/range_subselect.go | 1 + internal/sql/astutils/walk.go | 4 ++ 5 files changed, 70 insertions(+) diff --git a/internal/compiler/output_columns.go b/internal/compiler/output_columns.go index 65a2d5853c..f2b3d7d59b 100644 --- a/internal/compiler/output_columns.go +++ b/internal/compiler/output_columns.go @@ -413,6 +413,15 @@ func sourceTables(qc *QueryCatalog, node ast.Node) ([]*Table, error) { case *ast.RangeSubselect: cols, err := outputColumns(qc, n.Subquery) + // update columns if we are joining subquery + switch n.JoinType { + case ast.JoinTypeLeft: + fallthrough + case ast.JoinTypeFull: + for _, c := range cols { + c.NotNull = false + } + } if err != nil { return nil, err } diff --git a/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go b/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go index f59a55bf35..cf4a28a2cc 100644 --- a/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go +++ b/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go @@ -361,6 +361,51 @@ func (q *Queries) GetMayorsOptional(ctx context.Context) ([]GetMayorsOptionalRow return items, nil } +const getMayorsOptionalInnerSelect = `-- name: GetMayorsOptionalInnerSelect :many +SELECT t1.user_id, t2.full_name +FROM ( + SELECT user_id FROM users WHERE users.city_id = $1 LIMIT 1 OFFSET 0 +) AS t1 +LEFT JOIN cities on users.city_id = cities.city_id +LEFT JOIN ( + SELECT mayors.mayor_id, mayors.full_name + FROM mayors where mayors.mayor_id = $2 +) AS t2 on t2.mayor_id = cities.mayor_id +` + +type GetMayorsOptionalInnerSelectParams struct { + CityID sql.NullInt32 + MayorID int32 +} + +type GetMayorsOptionalInnerSelectRow struct { + UserID int32 + FullName sql.NullString +} + +func (q *Queries) GetMayorsOptionalInnerSelect(ctx context.Context, arg GetMayorsOptionalInnerSelectParams) ([]GetMayorsOptionalInnerSelectRow, error) { + rows, err := q.db.QueryContext(ctx, getMayorsOptionalInnerSelect, arg.CityID, arg.MayorID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []GetMayorsOptionalInnerSelectRow + for rows.Next() { + var i GetMayorsOptionalInnerSelectRow + if err := rows.Scan(&i.UserID, &i.FullName); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Close(); err != nil { + return nil, err + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const getSuggestedUsersByID = `-- name: GetSuggestedUsersByID :many SELECT DISTINCT u.user_id, u.user_nickname, u.user_email, u.user_display_name, u.user_password, u.user_google_id, u.user_apple_id, u.user_bio, u.user_created_at, u.user_avatar_id, m.media_id, m.media_created_at, m.media_hash, m.media_directory, m.media_author_id, m.media_width, m.media_height FROM users_2 u diff --git a/internal/endtoend/testdata/join_left/postgresql/query.sql b/internal/endtoend/testdata/join_left/postgresql/query.sql index 816f154aff..5151c5d9e4 100644 --- a/internal/endtoend/testdata/join_left/postgresql/query.sql +++ b/internal/endtoend/testdata/join_left/postgresql/query.sql @@ -111,3 +111,14 @@ FROM users_2 u ON u.user_avatar_id = m.media_id WHERE u.user_id != @user_id LIMIT @user_imit; + +-- name: GetMayorsOptionalInnerSelect :many +SELECT t1.user_id, t2.full_name +FROM ( + SELECT user_id FROM users WHERE users.city_id = $1 LIMIT 1 OFFSET 0 +) AS t1 +LEFT JOIN cities on users.city_id = cities.city_id +LEFT JOIN ( + SELECT mayors.mayor_id, mayors.full_name + FROM mayors where mayors.mayor_id = $2 +) AS t2 on t2.mayor_id = cities.mayor_id; diff --git a/internal/sql/ast/range_subselect.go b/internal/sql/ast/range_subselect.go index aaf4d5adaf..9c5c7453ca 100644 --- a/internal/sql/ast/range_subselect.go +++ b/internal/sql/ast/range_subselect.go @@ -2,6 +2,7 @@ package ast type RangeSubselect struct { Lateral bool + JoinType JoinType Subquery Node Alias *Alias } diff --git a/internal/sql/astutils/walk.go b/internal/sql/astutils/walk.go index eefad0ac03..8a12a334ad 100644 --- a/internal/sql/astutils/walk.go +++ b/internal/sql/astutils/walk.go @@ -1320,6 +1320,10 @@ func Walk(f Visitor, node ast.Node) { Walk(f, n.Larg) } if n.Rarg != nil { + switch t := n.Rarg.(type) { + case *ast.RangeSubselect: + t.JoinType = n.Jointype + } Walk(f, n.Rarg) } if n.UsingClause != nil { From fde2c8bf8d3630937608c856992c1de90a139777 Mon Sep 17 00:00:00 2001 From: Richard Bizik Date: Thu, 3 Mar 2022 18:52:43 +0100 Subject: [PATCH 2/3] Removed ast modifications, replaced with nullableNodes nullableNodes are used to identify their children and set columns to nullable --- internal/compiler/output_columns.go | 33 ++++++++++++++++++++--------- internal/sql/ast/range_subselect.go | 1 - internal/sql/astutils/search.go | 22 +++++++++++++++++++ internal/sql/astutils/walk.go | 4 ---- 4 files changed, 45 insertions(+), 15 deletions(-) diff --git a/internal/compiler/output_columns.go b/internal/compiler/output_columns.go index f2b3d7d59b..f8b7150435 100644 --- a/internal/compiler/output_columns.go +++ b/internal/compiler/output_columns.go @@ -359,6 +359,7 @@ func isTableRequired(n ast.Node, col *Column, prior int) int { // Return an error if an unknown column is referenced func sourceTables(qc *QueryCatalog, node ast.Node) ([]*Table, error) { var list *ast.List + var nullableNodes []*ast.Node switch n := node.(type) { case *ast.DeleteStmt: list = &ast.List{ @@ -369,14 +370,29 @@ func sourceTables(qc *QueryCatalog, node ast.Node) ([]*Table, error) { Items: []ast.Node{n.Relation}, } case *ast.SelectStmt: - list = astutils.Search(n.FromClause, func(node ast.Node) bool { - switch node.(type) { + dirtyList := astutils.Search(n.FromClause, func(node ast.Node) bool { + switch t := node.(type) { case *ast.RangeVar, *ast.RangeSubselect, *ast.FuncName: return true + case *ast.JoinExpr: + if t.Jointype == ast.JoinTypeLeft || t.Jointype == ast.JoinTypeFull { + return true + } + return false default: return false } }) + // split result into nullableJoins and list of nodes to handle + list = &ast.List{} + for i, v := range dirtyList.Items { + switch t := dirtyList.Items[i].(type) { + case *ast.JoinExpr: + nullableNodes = append(nullableNodes, &t.Rarg) + default: + list.Items = append(list.Items, v) + } + } case *ast.TruncateStmt: list = astutils.Search(n.Relations, func(node ast.Node) bool { _, ok := node.(*ast.RangeVar) @@ -413,18 +429,15 @@ func sourceTables(qc *QueryCatalog, node ast.Node) ([]*Table, error) { case *ast.RangeSubselect: cols, err := outputColumns(qc, n.Subquery) - // update columns if we are joining subquery - switch n.JoinType { - case ast.JoinTypeLeft: - fallthrough - case ast.JoinTypeFull: + + if err != nil { + return nil, err + } + if astutils.IsChildOfNodes(nullableNodes, &item) { for _, c := range cols { c.NotNull = false } } - if err != nil { - return nil, err - } tables = append(tables, &Table{ Rel: &ast.TableName{ Name: *n.Alias.Aliasname, diff --git a/internal/sql/ast/range_subselect.go b/internal/sql/ast/range_subselect.go index 9c5c7453ca..aaf4d5adaf 100644 --- a/internal/sql/ast/range_subselect.go +++ b/internal/sql/ast/range_subselect.go @@ -2,7 +2,6 @@ package ast type RangeSubselect struct { Lateral bool - JoinType JoinType Subquery Node Alias *Alias } diff --git a/internal/sql/astutils/search.go b/internal/sql/astutils/search.go index 5aeacfb9d9..7d1995ce4c 100644 --- a/internal/sql/astutils/search.go +++ b/internal/sql/astutils/search.go @@ -19,3 +19,25 @@ func Search(root ast.Node, f func(ast.Node) bool) *ast.List { Walk(ns, root) return ns.list } + +func IsChildOfNodes(parents []*ast.Node, node *ast.Node) bool { + for _, v := range parents { + if IsChildOfNode(v, node) { + return true + } + } + return false +} + +func IsChildOfNode(parent *ast.Node, node *ast.Node) bool { + res := Search(*parent, func(n ast.Node) bool { + if n == *node { + return true + } + return false + }) + if len(res.Items) > 0 { + return true + } + return false +} diff --git a/internal/sql/astutils/walk.go b/internal/sql/astutils/walk.go index 8a12a334ad..eefad0ac03 100644 --- a/internal/sql/astutils/walk.go +++ b/internal/sql/astutils/walk.go @@ -1320,10 +1320,6 @@ func Walk(f Visitor, node ast.Node) { Walk(f, n.Larg) } if n.Rarg != nil { - switch t := n.Rarg.(type) { - case *ast.RangeSubselect: - t.JoinType = n.Jointype - } Walk(f, n.Rarg) } if n.UsingClause != nil { From 4caff5ef6d0a6a72c2f9c2dac7876a505fe28f2d Mon Sep 17 00:00:00 2001 From: Richard Bizik Date: Wed, 16 Mar 2022 13:06:08 +0100 Subject: [PATCH 3/3] Fixed nested query --- .../endtoend/testdata/join_left/postgresql/go/query.sql.go | 4 ++-- internal/endtoend/testdata/join_left/postgresql/query.sql | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go b/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go index cf4a28a2cc..a78277b3f2 100644 --- a/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go +++ b/internal/endtoend/testdata/join_left/postgresql/go/query.sql.go @@ -364,9 +364,9 @@ func (q *Queries) GetMayorsOptional(ctx context.Context) ([]GetMayorsOptionalRow const getMayorsOptionalInnerSelect = `-- name: GetMayorsOptionalInnerSelect :many SELECT t1.user_id, t2.full_name FROM ( - SELECT user_id FROM users WHERE users.city_id = $1 LIMIT 1 OFFSET 0 + SELECT user_id, city_id FROM users WHERE users.city_id = $1 LIMIT 1 OFFSET 0 ) AS t1 -LEFT JOIN cities on users.city_id = cities.city_id +LEFT JOIN cities on t1.city_id = cities.city_id LEFT JOIN ( SELECT mayors.mayor_id, mayors.full_name FROM mayors where mayors.mayor_id = $2 diff --git a/internal/endtoend/testdata/join_left/postgresql/query.sql b/internal/endtoend/testdata/join_left/postgresql/query.sql index 5151c5d9e4..f099d988ca 100644 --- a/internal/endtoend/testdata/join_left/postgresql/query.sql +++ b/internal/endtoend/testdata/join_left/postgresql/query.sql @@ -115,9 +115,9 @@ LIMIT @user_imit; -- name: GetMayorsOptionalInnerSelect :many SELECT t1.user_id, t2.full_name FROM ( - SELECT user_id FROM users WHERE users.city_id = $1 LIMIT 1 OFFSET 0 + SELECT user_id, city_id FROM users WHERE users.city_id = $1 LIMIT 1 OFFSET 0 ) AS t1 -LEFT JOIN cities on users.city_id = cities.city_id +LEFT JOIN cities on t1.city_id = cities.city_id LEFT JOIN ( SELECT mayors.mayor_id, mayors.full_name FROM mayors where mayors.mayor_id = $2