8000 Fix hstore SQL literal generation for null and escaped values by brianpursley · Pull Request #3780 · npgsql/efcore.pg · GitHub
[go: up one dir, main page]
Skip to content

Fix hstore SQL literal generation for null and escaped values#3780

Open
brianpursley wants to merge 3 commits intonpgsql:mainfrom
brianpursley:hstore-fix
Open

Fix hstore SQL literal generation for null and escaped values#3780
brianpursley wants to merge 3 commits intonpgsql:mainfrom
brianpursley:hstore-fix

Conversation

@brianpursley
Copy link
Contributor
@brianpursley brianpursley commented Mar 12, 2026

Summary

Fix hstore SQL literal generation bugs in NpgsqlHstoreTypeMapping.

Details

The previous implementation produced invalid or corrupted literals in several cases:

  • A null-valued entry would not have the separating comma emitted leading to unexpected results
  • Empty dictionaries produced an unterminated literal
  • Keys/values containing special characters were not escaped correctly

This PR updates hstore literal generation to:

  • Emit separators independently of null handling
  • Handle empty dictionaries correctly
  • Escape \, " and ' appropriately for use inside the generated SQL literal

Unit tests were added for each of these fixed cases.

Copilot AI review requested due to automatic review settings March 12, 2026 21:55
Copilot AI reviewed Mar 12, 2026
View reviewed changes
Copy link
Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes SQL literal generation for PostgreSQL hstore in NpgsqlHstoreTypeMapping to correctly handle null values, empty dictionaries, and escaping of special characters, with unit tests covering the corrected behavior.

Changes:

  • Reworked hstore SQL literal generation to emit separators correctly and support empty dictionaries.
  • Added proper escaping for \, " and ' when generating hstore SQL literals.
  • Added unit tests for null values, escaping, and empty dictionaries.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
src/EFCore.PG/Storage/Internal/Mapping/NpgsqlHstoreTypeMapping.cs Fixes hstore SQL literal formatting/escaping and handles empty dictionaries safely.
test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs Adds unit test coverage for the fixed hstore literal generation edge cases.

@brianpursley
Update test/EFCore.PG.Tests/Storage/NpgsqlTypeMappingTest.cs
f1eac90 
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 12, 2026 22:14
Copilot started reviewing on behalf of brianpursley March 12, 2026 22:15 View session
Copilot AI reviewed Mar 12, 2026
View reviewed changes
Copy link
Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

@brianpursley
Copy link
Contributor Author
brianpursley commented Mar 12, 2026

Here is a simple Program.cs that exercises the changes:

using var dbContext = new ExampleDbContext();
dbContext.Database.EnsureDeleted();
dbContext.Database.EnsureCreated();

dbContext.Blogs.Add(new Blog { Name = "ABC"});
dbContext.SaveChanges();

var blog = dbContext.Blogs.Single(b => b.Name == "ABC");

foreach (var item in blog.Features!)
{
    Console.WriteLine($"{item.Key}: {item.Value ?? "<NULL>"}");
}

public class ExampleDbContext : DbContext
{
    public DbSet<Blog> Blogs { get; set; }

    protected override void OnModelCreating(ModelBuilder modelBuilder)
    {
        modelBuilder.HasPostgresExtension("hstore");
        modelBuilder.Entity<Blog>()
            .Property(x => x.Features)
            .HasDefaultValue(new Dictionary<string, string?>
            {
                ["k1"] = null,
                ["k\"2\""] = "v\"2\"",
                ["k'3'"] = "v'3'",
                ["k\\4"] = "v\\4"
            });
    }
    
    protected override void OnConfiguring(DbContextOptionsBuilder optionsBuilder)
    {
        optionsBuilder
            .UseNpgsql("Host=localhost;Username=postgres;Password=postgres;Database=ExampleDb")
            .LogTo(Console.WriteLine, LogLevel.Information)
            .EnableSensitiveDataLogging();
    }
}

public class Blog
{
    public int Id { get; set; }
    public required string Name { get; set; }
    public Dictionary<string, string?>? Features { get; set; } 
}

Logged table creation:

CREATE TABLE "Blogs" (
    "Id" integer GENERATED BY DEFAULT AS IDENTITY,
    "Name" text NOT NULL,
    "Features" hstore DEFAULT HSTORE '"k1"=>NULL,"k\"2\""=>"v\"2\"","k''3''"=>"v''3''","k\\4"=>"v\\4"',
    CONSTRAINT "PK_Blogs" PRIMARY KEY ("Id")
);

Output:

k1: <NULL>
k\4: v\4
k"2": v"2"
k'3': v'3'

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

Reviewers

Copilot code review Copilot Copilot left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@brianpursley
0