Skip to content

bug: Adding a DuplicateField/Index to a List<T> column generates invalid code.#3702

Closed
elexisvenator wants to merge 4 commits intoJasperFx:masterfrom
elexisvenator:list-array-dbtype-codegen
Closed

bug: Adding a DuplicateField/Index to a List<T> column generates invalid code.#3702
elexisvenator wants to merge 4 commits intoJasperFx:masterfrom
elexisvenator:list-array-dbtype-codegen

Conversation

@elexisvenator
Copy link
Copy Markdown
Contributor

@elexisvenator elexisvenator commented Mar 5, 2025

This is one of the things I found while putting this test app together.

There is hardcoded magic to make array types work correctly as duplicate fields and indexes. But that doesnt apply to other collection types like lists.

A table with a list type as follows:

public class DocWithIndexOnList
{
    public Guid Id { get; set; }

    [DuplicateField(DbType = NpgsqlDbType.Array | NpgsqlDbType.Text, PgType = "text[]", IndexMethod = IndexMethod.gin)]
    public List<string> ListOfStrings { get; set; }
}

Will generate invalid code:

public override void ConfigureParameters(IGroupedParameterBuilder parameterBuilder, ICommandBuilder builder, DocWithIndexOnList document, IMartenSession session)
{
    builder.Append("select bugs.mt_upsert_bug_pr_xxxx_list_index_compile_error_docwithindexonlist(");

    if (document.ListOfStrings != null)
    {
        var parameter0 = parameterBuilder.AppendParameter(document.ListOfStrings);
        parameter0.NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.-2147483629;

Arrays in NpgsqlDbType are a dirty hack on enums, so the best way to fix this issue is to always cast the integer to the enum.

The one thing this leaves is the magic handling of array types - its kindof ok but it will override a manually set dbType, that part doesn't feel right.

@elexisvenator
Copy link
Copy Markdown
Contributor Author

Not sure if related, but we are also having issues with the following duplicate field declaration:

[DuplicateField(IndexMethod = IndexMethod.gin)]
public string[] ChildReferenceFormIds { get; init; } 

The error we are getting is Npgsql.PostgresException: '42804: binary data has array element type 25 (text) instead of expected 1043 (character varying)', which I think is happening on either storing documents or querying documents (likely storing)

jeremydmiller added a commit to JasperFx/jasperfx that referenced this pull request Apr 28, 2026
The previous implementation rendered any enum value as
$"{Type}.{value}", which silently calls Enum.ToString().
For values that don't correspond to a single named member —
most notably bit-OR'd combinations on non-[Flags] enums like
Npgsql.NpgsqlDbType — ToString returns the underlying integer
literal as a string, so we'd emit uncompilable source such as
`NpgsqlTypes.NpgsqlDbType.-2147483629`. Roslyn then rejects the
generated assembly with CS1001.

Replace the single-line implementation with WriteEnum, which
handles three cases:

  1. Defined single member  → Type.Name (unchanged)
  2. [Flags] combination     → Type.A | Type.B
  3. Undefined value         → ((Type)(rawIntegerLiteral))

Three new CodeFormatterTests cover each branch, including the
exact non-[Flags] OR'd-bit shape that NpgsqlDbType uses.

Fixes the Marten codegen bug tracked in JasperFx/marten#3702.
@jeremydmiller
Copy link
Copy Markdown
Member

Hi @elexisvenator — thanks again for the reproduction case. Digging into this now I think the cleanest fix lives one layer down from UpsertArgument.

The reason NpgsqlDbType.Array | NpgsqlDbType.Text rendered as NpgsqlTypes.NpgsqlDbType.-2147483629 is that JasperFx's CodeFormatter.Write was emitting any enum as Type.{value}, which calls Enum.ToString() — and for an undefined value (like a bit-OR combination on a non-[Flags] enum), that returns the integer literal as a string. So every call to Constant.ForEnum was vulnerable, not just the two sites in UpsertArgument.

I've put the fix in CodeFormatter.Write itself in JasperFx/jasperfx#197 (now merged + published as JasperFx 1.28.2):

  • Defined single member → Type.Name (unchanged)
  • [Flags] combo → Type.A | Type.B
  • Undefined value → ((Type)(rawIntegerLiteral))

Marten just needs to bump JasperFx; #4314 does that and pulls your reproduction test in verbatim (you're attributed as co-author on that commit). Going to close this PR in favour of #4314 once that lands — but the test is preserved and the bug stays fixed.

Thanks for the careful reproduction; "always cast the integer to the enum" was exactly the right intuition.

jeremydmiller added a commit that referenced this pull request Apr 29, 2026
…#3702) (#4314)

JasperFx 1.28.2 includes a fix to CodeFormatter.Write that emits
safe C# for any enum value — including bit-OR'd combinations on
non-[Flags] enums like Npgsql.NpgsqlDbType. Previously a member
declared as

    [DuplicateField(DbType = NpgsqlDbType.Array | NpgsqlDbType.Text,
                    PgType = "text[]", IndexMethod = IndexMethod.gin)]
    public List<string> ListOfStrings { get; set; }

would generate uncompilable source like

    parameter0.NpgsqlDbType = NpgsqlTypes.NpgsqlDbType.-2147483629;

because Enum.ToString returns the integer literal as a string for
undefined enum values. The JasperFx fix emits a checked cast for
those values: `((NpgsqlDbType)(-2147483629))`.

The reproduction test from elexisvenator's #3702 is preserved here
verbatim; the JasperFx-side fix lets it pass with no Marten code
change. Closes #3702.

Co-authored-by: Ben Edwards <elexisvenator@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@jeremydmiller
Copy link
Copy Markdown
Member

Landed via #4314 (merge commit 9898bbc). Thanks again @elexisvenator!

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants