Skip to content

Reject database NULL for non-nullable model columns consistently across all read paths #11

Description

@bazer

Reject database NULL for non-nullable model columns consistently across all read paths

Summary

DataLinq does not enforce its model nullability contract consistently across its SQL read paths.

When a database column contains NULL while the DataLinq model declares the property/column as non-nullable, the
failure can currently surface in three materially different ways:

  1. the canonical v0.9 materialization path throws a column-aware decoding/materialization exception,
  2. legacy RowData(IDataLinqDataReader, ...) paths store null and a later generated property getter throws the
    misleading ArgumentNullException: Value cannot be null. (Parameter 'columnIndex'), or
  3. direct IDataLinqDataReader.GetValue<T>(...) calls return default(T) for SQL NULL, silently changing values such
    as char, DateTime, numeric types, or bool into valid-looking CLR defaults.

The second behavior makes a schema/model nullability mismatch look like an invalid column ordinal or damaged row
shape. The third behavior can hide the mismatch completely and corrupt application semantics.

Verified against commit 3854927d323f55d38e4d9c1f5b0d98441accd071 on branch v0.9. The misleading getter behavior
also exists in DataLinq 0.7.1.

Current behavior

Immutable.GetValue(int columnIndex) still throws using the index parameter name:

protected object GetValue(int columnIndex) =>
    GetNullableValue(columnIndex) ?? throw new ArgumentNullException(nameof(columnIndex));

The index is valid in this failure mode. The value read from that column is null, so columnIndex is not the bad
argument and the exception sends diagnosis in the wrong direction.

Both the MySQL/MariaDB and SQLite readers currently return default before considering the column's declared
nullability or requested CLR type:

if (IsDbNull(ordinal))
    return default;

The public RowData(IDataLinqDataReader, ...) constructor uses these generic reader methods directly and does not
validate the value against ColumnDefinition.Nullable or ValueProperty.CsNullable before storing it.

The newer ProviderRowDecoder/CanonicalProviderValueRow path is better: it represents SQL NULL as null, checks
the column metadata, and includes table.column plus expected provider type in the resulting diagnostic. However,
not every read path uses that boundary. For example, the current code still constructs RowData directly from a
reader in legacy query, transaction, select, and cache-loading paths.

Minimal reproduction shape

  1. Create a table with a nullable column, for example status CHAR(1) NULL or created_at DATETIME NULL.
  2. Map it to a generated DataLinq property without [Nullable], for example char Status or
    DateTime CreatedAt.
  3. Insert a row where the column is SQL NULL.
  4. Read the row through each supported model-loading path and through IDataLinqDataReader.GetValue<T>.
  5. Access the generated non-nullable property.

Observed historically on 0.7.1 through normal model loading:

System.ArgumentNullException: Value cannot be null. (Parameter 'columnIndex')

Observed through direct generic reader access for value types: default(T) is returned, for example \0 for
char or DateTime.MinValue for DateTime.

Expected behavior

SQL NULL for a model column declared non-nullable should fail immediately at the shared reader/materialization
boundary on every provider and every public model-loading path.

The failure should:

  • use a public, DataLinq-owned exception type that consumers can catch without matching message text,
  • identify the database table and column and, when available, the generated model property,
  • state that SQL returned NULL while the DataLinq model declares the value non-nullable,
  • include a non-sensitive logical source/read-path label,
  • suggest checking schema drift or marking the model property nullable,
  • not include the actual database value or connection details.

Direct generic reader behavior should be explicit as well: return null only when the requested/model contract permits
it; otherwise throw the same focused exception rather than returning a valid-looking CLR default.

Suggested implementation direction

  • Route all model-row construction through the shared ProviderRowDecoder and ProviderRowMaterializer contract, or
    apply equivalent validation inside the remaining RowData(IDataLinqDataReader, ...) constructor paths.
  • Make the focused decoding/materialization exception public, or expose a public stable superclass containing table,
    column, property, source, and mismatch kind.
  • Replace the ArgumentNullException(nameof(columnIndex)) fallback. A null database value is not an invalid index
    argument.
  • Define and document the null contract of IDataLinqDataReader.GetValue<T> for reference types, nullable value types,
    and non-nullable value types.

Acceptance criteria

  • MySQL, MariaDB, and SQLite produce the same focused failure class and equivalent safe context for SQL NULL read
    against a non-nullable model column.
  • Coverage includes non-nullable reference and value-type properties.
  • Coverage includes normal query roots, read-only/raw-command model loading, transaction loading, relation/cache
    loading, direct projections, and direct IDataLinqDataReader.GetValue<T> access where those are supported APIs.
  • No path reports columnIndex as the null argument for this mismatch.
  • No path silently converts SQL NULL to default(T) for a non-nullable model value.
  • Nullable model columns continue to materialize SQL NULL as null.
  • Diagnostics do not expose the database value or connection details.

Related prevention already available in v0.9

datalinq validate already compares live provider metadata with generated model metadata, including column
nullability. That is valuable CI prevention, but it does not replace deterministic runtime behavior because deployed
schema drift and legacy databases can still occur.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions