Subselected value should be nullable

[aarondl0]

Version

1.10.0

What happened?

-- name: ShouldHaveNull :many
select
   (select other_table.name from other_table where this_table.name = other_table.name) as "name"
from this_table;

sqlc will generate an output struct like this:

type ShouldHaveNullRow struct {
  Name string
}

But the struct should be:

type ShouldHaveNullRow struct {
  Name sql.NullString
}

The above example is with strings but I thought it was relevant to mention that this is happening with a replaced type that's configured similarly to:

  - db_type: "pg_catalog.numeric"
    nullable: false
    go_type:
      import: "github.com/shopspring/decimal"
      type: "Decimal"
      pointer: false
  - db_type: "pg_catalog.numeric"
    nullable: true
    go_type:
      import: "github.com/shopspring/decimal"
      type: "NullDecimal"
      pointer: false

I know the types in this example are all messed up. I'm in a huge rush and don't have time to file the issue with extreme precision at this moment.

You can see in the playground link because the type is non nullable in the table, it incorrectly assumes that the subselect can never return null for that field.

Relevant log output

No response

Database schema

No response

SQL queries

No response

Configuration

No response

Playground URL

https://play.sqlc.dev/p/bb47e4143c86e1b45bc659b147c4072d0464d968332d8598e36f7c2e3e531f18

What operating system are you using?

Linux

What database engines are you using?

PostgreSQL

What type of code are you generating?

Go

[kyleconroy]

I'm not able to reproduce this error. The playground link you provided is working as intended.

I created a smaller example (https://play.sqlc.dev/p/984e6f38c34bd8ef7ea26c9e246738f38fd993e90b1f105cdf89fe185cb55a48) and it's working as intended as well.

[aarondl0]

I think you may be misunderstanding the complaint.

Let me explain more clearly (I have a bit more time now). There is a subselect query happening where we're retrieving a single value from it:

(select o.name from one o where one.name = 5 limit 1) as "name"

If you look at the struct it created to capture the result of Test query in the playground it is:

type TestRow struct {
	Otherfield sql.NullString
	Name       decimal.Decimal
}

However it should be:

type TestRow struct {
	Otherfield sql.NullString
	Name       decimal.NullDecimal // note the difference here
}

The first struct (the one sqlc generates and is in my playground link) is incorrect. This valid query cannot properly be bound to this struct for all possible return values (as the subquery can return null). Does that make it more clear?

Your example is working I agree. However I could only reproduce when using type overrides which your example does not include. Please take another look.

[voluntas]

I am also having the same problem. Even if I reflect the type in sqlc.yml in the subquery results, it is not applied. Could you please check this Issue again?

[WesleyMiller1998]

@voluntas pinged me on this issue, but I cannot find where the problem is here. I am having no problems generating the correct return types. I think there needs to be better repro instructions

[jakoguta]

@WesleyMiller1998 I agree this issue configuration and generation seems to works fine; as the override is done at the database data type. However, your issue #1752 the override requested is by column name and it does have a bug. @kyleconroy I propose this issue to be closed.

[aarondl0]

@jakoguta @WesleyMiller1998 this issue is still a real issue. There's a clear reproduce here in my original playground link: https://play.sqlc.dev/p/bb47e4143c86e1b45bc659b147c4072d0464d968332d8598e36f7c2e3e531f18

Note the struct:

type TestRow struct {
	Otherfield sql.NullString
	Name       decimal.Decimal
}

Until Name is generated as a decimal.NullDecimal (as per the config for the nullable decimal type) there is a bug here as the Name field in the query is coming from a subselect which can be null. See the query here:

-- name: Test :many
select 
	otherfield,
	(select o.name from one o where one.name = 5 limit 1) as "name"
from one;

In the simplest case, if the one table has one row where .name is not 5, this query will run fine in postgres and return results, and then the results should fail to bind properly because null will be returned and attempt to scan into Name decimal.Decimal which isn't a valid value for a decimal.Decimal.

Kyle's response earlier in the thread tries to show that there is "no bug" by creating a smaller reproduce but in doing so avoids the bug altogether because the bug seems contingent on having replaced types in the config file (as I have in my playground link).

Let me know if further explanation is required but let's keep the issue opened so long as sqlc is generating structs that cannot be bound to all result sets from a given query.

Edit: Sorry, wrote the wrong struct in the above. Corrected.

[WesleyMiller1998]

@aarondl0 Thank you for the response I'm understanding the problem much better now and you are absolutely correct that it is generating the wrong type. I've spent a good hour digging around in the code and I think I found the issue, but I am not entirely sure yet. I will update later this week

[WesleyMiller1998]

Okay, as I thought I located the issue. It has to do with the follow lines of code: https://github.com/kyleconroy/sqlc/blob/0ae16d272c715c8ef91197512bef98e34b03fbe2/internal/compiler/output_columns.go#L230-L250

With this particular example, we hit the EXPR_SUBLINK switch case where we then recursively determine the output columns of the select statement. I created a PR: #1758 which does allow for type generation to occur as expected. I can't see any obvious negative impact from this, but I would really like some general thoughts on if this appropriate.

The PR basically just makes any subquery have a nullable type by default. I believe this is the expected behavior but I could be wrong.

Edit: I'm a baboon and didn't finish my thought

[voluntas]

@aarondl0 Thank you for the very detailed way to reproduce this. I am experiencing the same problem.