Add Proper Nullable support to Procedure Definitions

[jwyza-pi]

What problem are you trying to solve?

Back in #2261, it was made that the generated code for Stored Procedures was always set to Nullable disable.

However, the real solution should be (imo) properly supporting nullable. If "use-nullable-reference-types" is set to true, then the parameters for generated Sprocs should augmented with the ? so that they are classified as nullable and the "#nullable disable" should not be part of the Procedures class.

eg:

 internal partial class MyDbContextProcedures : IMyDbContextProcedures
{
    private readonly MyDbContext _context;

    public MyDbContextProcedures(MyDbContext context)
    {
        _context = context;
    }

    public virtual async Task<List<GetMyWidgetResult>> GetMyWidgetAsync(int? id, OutputParameter<int>? returnValue = null, CancellationToken? cancellationToken = default)
    {
        var parameterreturnValue = new SqlParameter
        {
            ParameterName = "returnValue",
            Direction = System.Data.ParameterDirection.Output,
            SqlDbType = System.Data.SqlDbType.Int,
        };

        var sqlParameters = new []
        {
            new SqlParameter
            {
                ParameterName = "Id",
                Value = id ?? Convert.DBNull,
                SqlDbType = System.Data.SqlDbType.Int,
            },
            parameterreturnValue,
        };
        var _ = await _context.SqlQueryAsync<GetMyWidgetResult>("EXEC @returnValue = [dbo].[GetMyWidget] @id = @id", sqlParameters, cancellationToken);

        returnValue?.SetValue(parameterreturnValue.Value);

        return _;
    }
}

specifically this line would change:

    public virtual async Task<List<GetMyWidgetResult>> GetMyWidgetAsync(int? id, OutputParameter<int> returnValue = null, CancellationToken cancellationToken = default)

to

    public virtual async Task<List<GetMyWidgetResult>> GetMyWidgetAsync(int? id, OutputParameter<int>? returnValue = null, CancellationToken? cancellationToken = default)

The main reason is that right now if the rest of your project uses NRTs, then this shows up as a warning:

ISearchDbContextProcedures procs; // injected
var result = await procs.GetMyWidgetAsync(1);
var first = result.SingleOrDefault(); // Warning since it can't know that result will never be null due to nullable disable being set. 

You can obviously use the null forgiveness operator (!), but that's not the best practice if it can be avoided.

Describe the solution you'd like

No response

[ErikEJ]

Makes sense, since 6 (8) is the current supported .net versions.

[spaasis]

Yeah this behavior is not the best. I could try having a go at this...

Am I correct in inferring that the ultimately the parameter types (for postgres) come from PostgresRoutineModelFactory? So if that query is updated to fetch the nullability information, it should be possible to route it back to the GenerateProcedure in PostgresStoredProcedureScaffolder, right?

[ErikEJ]

@spaasis This issue is not about the nullability of parameters and result set properties, but the fact that these pieces of code uses #nullable disable (to be backwards compatible) - this is no longer needed/useful.

Please create a new issue to improve nullability discovery for postgres schemas (which is what you are referring to, and yes, PostgresRoutineModelfacory is responsible for that discovery)

[spaasis]

@ErikEJ apologies, misunderstood the opening

[ErikEJ]

@jwyza-pi Is this something you are interested in contributing? 🙏

[jwyza-pi]

Interested? Yes. But unfortunately, I won't have the capacity to do it until probably sometime in mid-June. :(