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

Arguments to SQL functions are nullable by default #940

Open
breml opened this issue Mar 11, 2021 · 4 comments
Open

Arguments to SQL functions are nullable by default #940

breml opened this issue Mar 11, 2021 · 4 comments
Labels
📚 postgresql enhancement New feature or request future In the year 3000... 🔧 golang

Comments

@breml
Copy link

breml commented Mar 11, 2021

While working on a solution for #364 which is based on SQL functions, I hit the following problem.

Arguments for SQL functions in PostgreSQL are nullable by default. With CALLED ON NULL INPUT, this can even be explicitly stated, but this is ignored by sqlc.

Given the following definition for PostgreSQL:

CREATE TABLE foo (
            id     INTEGER,
            bar    varchar(100)
);

INSERT INTO foo VALUES (null, 'foo'), (1, 'bar');

CREATE OR REPLACE FUNCTION select1(_id INTEGER)
  RETURNS SETOF foo as
$func$
BEGIN
  IF _id IS NULL THEN
    RETURN QUERY EXECUTE 'select * from foo where id IS NULL';
  ELSE
    RETURN QUERY EXECUTE FORMAT('select * from foo where id = %L', _id);
  END IF;
END
$func$ LANGUAGE plpgsql CALLED ON NULL INPUT;

the following SQL calls are valid:

select select1(null);
select select1(1);

With the query definition:

-- name: GetSelect1 :many
SELECT select1($1);

the following Go method signature is generated:

func (q *Queries) GetSelect1(ctx context.Context, ID int32) ([]interface{}, error) {

With this method signature it is not possible to pass nil (or SQL null) as argument for ID.

Update: fix typo

@breml
Copy link
Author

breml commented Mar 17, 2021

I looked into this in a more detail and it looks like the underlying github.com/lfittl/pg_query_go does not return the correct information.

Given the following example:

package main

import (
	"fmt"

	pg_query "github.com/lfittl/pg_query_go"
)

func main() {
	tree, err := pg_query.ParseToJSON(`CREATE OR REPLACE FUNCTION select1(_id INTEGER)
  RETURNS SETOF foo as
$func$
BEGIN
  IF _id IS NULL THEN
    RETURN QUERY EXECUTE 'select * from foo where id IS NULL';
  ELSE
    RETURN QUERY EXECUTE FORMAT('select * from foo where id = %L', _id);
  END IF;
END
$func$ LANGUAGE plpgsql CALLED ON NULL INPUT;`)
	if err != nil {
		panic(err)
	}
	fmt.Printf("%s\n", tree)
}

I get this output:

[
  {
    "RawStmt": {
      "stmt": {
        "CreateFunctionStmt": {
          "replace": true,
          "funcname": [
            {
              "String": {
                "str": "select1"
              }
            }
          ],
          "parameters": [
            {
              "FunctionParameter": {
                "name": "_id",
                "argType": {
                  "TypeName": {
                    "names": [
                      {
                        "String": {
                          "str": "pg_catalog"
                        }
                      },
                      {
                        "String": {
                          "str": "int4"
                        }
                      }
                    ],
                    "typemod": -1,
                    "location": 39
                  }
                },
                "mode": 105
              }
            }
          ],
          "returnType": {
            "TypeName": {
              "names": [
                {
                  "String": {
                    "str": "foo"
                  }
                }
              ],
              "setof": true,
              "typemod": -1,
              "location": 64
            }
          },
          "options": [
            {
              "DefElem": {
                "defname": "as",
                "arg": [
                  {
                    "String": {
                      "str": "\nBEGIN\n  IF _id IS NULL THEN\n    RETURN QUERY EXECUTE 'select * from foo where id IS NULL';\n  ELSE\n    RETURN QUERY EXECUTE FORMAT('select * from foo where id = %L', _id);\n  END IF;\nEND\n"
                    }
                  }
                ],
                "defaction": 0,
                "location": 68
              }
            },
            {
              "DefElem": {
                "defname": "language",
                "arg": {
                  "String": {
                    "str": "plpgsql"
                  }
                },
                "defaction": 0,
                "location": 270
              }
            },
            {
              "DefElem": {
                "defname": "strict",
                "arg": {
                  "Integer": {
                    "ival": 0
                  }
                },
                "defaction": 0,
                "location": 287
              }
            }
          ]
        }
      },
      "stmt_len": 307
    }
  }
]

The interesting part is the last DefElem, which is strict, but we would expect it to be called on null input.

@breml
Copy link
Author

breml commented Mar 17, 2021

In order to allow nullable inputs for sql functions, the this line (https://github.com/kyleconroy/sqlc/blob/master/internal/compiler/resolve.go#L267) would needed to be changed returning false instead of true.
But this change obviously breaks several unit tests as well as it would break the code for all existing users. So the question is, what is the best way to fix this. Either some sort of configuration or the above mentioned breaking change.

@Yiwen-Gao
Copy link

I think the latest release v1.14.0 contains #1536, which provides the option to specify nullable arguments with sqlc.narg().

For your use case, it might work to use:

SELECT select1(sqlc.narg(_id)::integer);

which would generate sql.NullInt32 as the Go type for _id.

@kyleconroy kyleconroy added future In the year 3000... and removed analyzer labels Oct 16, 2023
@kyleconroy
Copy link
Collaborator

I believe that both sqlc.narg and #2800 solve this issue. We'll also want to revisit the default nullability at some point in the future, but it's a backwards incompatible change right now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 postgresql enhancement New feature or request future In the year 3000... 🔧 golang
Projects
None yet
Development

No branches or pull requests

3 participants