ConduitIO / ConduitIO/conduit-connector-postgres

Avro schema extraction emits non-nullable field types for any nullable Postgres column (not just bytes-backed logical types)

Open
#326 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
19
Forks
12
Avg merge
12h 20m
Merged PRs (30d)
5

Description

## Summary

Avro schema extraction (`source/schema/avro.go`) emits a non-nullable Avro field type for
**every** Postgres column, regardless of type - there is no nullable-vs-not-null distinction
anywhere in the extractor. Any nullable column, of any type, breaks the connector's own Avro
encoding the moment it actually contains a `NULL`. With `logrepl.withAvroSchema` defaulting to
`true` (`source/config.go:81`), this is a shipped-default bug, not an edge case, and it is not
limited to bytes-backed logical types (numeric/decimal) as originally filed - see "Scope
correction" below.

## Reproduction

Enable Avro schema attachment against the repo's own standard integration test table
(`test.SetupTestTableWithName`, `test/helper.go`) and read a row where `column4` (a nullable
`numeric(16,3)`) is `NULL`:

```
column4: avro: *avro.null is unsupported for Avro bytes
```

Delete that row and rerun with the same `withAvroSchema=true`: the failure moves to a plain
`int` column instead of disappearing, proving the bug is not specific to the bytes-backed
`numeric`/`decimal` logical type:

```
UppercaseColumn1: avro: *avro.null is unsupported for Avro int
```

`UppercaseColumn1` is a nullable `integer` column with no bytes-backed logical type involved at
all - see `test/helper.go`'s `testTableCreateQuery`.

## Root cause

`source/schema/avro.go`'s `avroExtractor.Extract` (called from both `ExtractLogrepl` and
directly) builds one Avro field per Postgres column via `avro.NewField(f.Name, s)`, where `s`
comes from `extractType`. That function returns either a fixed primitive schema from `avroMap`
(covering `bool`, `bytea`, `float4`/`float8`, `int2`/`int4`/`int8`, `text`, `varchar`, `jsonb`,
`json`, `timestamptz`, `timestamp`, `date`, `uuid`) or, for `numeric`, a bare
`{"type":"bytes","logicalType":"decimal",...}` - **never** a nullable union
(`["null", {...}]`) for any of them, and there is no code path anywhere in the extractor that
branches on nullability.

That is a structural gap, not a missed case for one type: the extractor's only inputs are
`pgconn.FieldDescription` (`Extract`) and `pglogrepl.RelationMessageColumn` (`ExtractLogrepl`
converts one to the other), and **neither struct carries a nullability flag at all** -
`pgconn.FieldDescription` has `Name`, `TableOID`, `TableAttributeNumber`, `DataTypeOID`,
`DataTypeSize`, `TypeModifier`, `Format`; `pglogrepl.RelationMessageColumn` has `Flags` (only
ever 0 or "is part of the replica identity key"), `Name`, `DataType`, `TypeModifier`. The
extractor cannot emit a nullable union today even for the one type it does special-case
(`numeric`), because it is never told whether the source column is nullable in the first place.

`test/helper.go`'s `testTableCreateQuery`/`TestTableAvroSchemaV1` (declaring `column4
numeric(16,3)` nullable, but its corresponding Avro fixture as a bare, non-union
`{"type":"bytes","logicalType":"decimal",...}`) is a **symptom fixture**, encoding the same
buggy shape the extractor produces - not the root cause. The original version of this issue
named `test/helper.go` as the root cause; that was a mis-scoping, corrected here.

## Scope correction (was: "bytes-backed logical types e.g. numeric")

This is **not** limited to bytes-backed logical types. It is any nullable column of any type
this extractor knows how to map at all - proved above by deleting the NULL `column4` row and
still getting a failure, on a plain `int` column (`UppercaseColumn1`). The original filing's
"at minimum numeric/decimal, likely other bytes-backed logical types too" undersold the actual
blast radius: every entry in `avroMap`, plus the hand-rolled `numeric` case, is affected the
same way, because none of them consult nullability.

## Why this matters beyond the harness

`WithAvroSchema` defaults to `true` (`source/config.go:81`), so this is the connector's
out-of-the-box behavior against its own standard nullable test columns, not a narrow
misconfiguration. Any production table with a nullable column of essentially any supported
type that ever actually contains a `NULL` value will hit this.

This was found while building the DBZ-3 process-kill chaos harness (`test/chaos`, PR #325): the
harness's smoke test runs `logrepl.withAvroSchema=false` specifically to route around this bug
and stay focused on `Ack`/batch/handoff behavior, tracked at `test/chaos/child.go`'s
`withAvroSchema: "false"` line and its `TODO(#326)`.

## Suggested fix direction

`avroExtractor.Extract`/`ExtractLogrepl` need an input that actually carries nullability -
neither `pgconn.FieldDescription` nor `pglogrepl.RelationMessageColumn` has it today, so this
likely needs either a schema lookup against `information_schema.columns`/`pg_attribute`
(`attnotnull`) at extraction time, or a nullability parameter threaded in by the caller that
already knows it. Once available, wrap every field's schema in a nullable union (`["null",
{...}]`, with `"default": null`) for every column that is not `NOT NULL`, not just the `numeric`
special case. `TestTableAvroSchemaV1`/`V2` in `test/helper.go` should be updated to match once
the underlying extraction is fixed, since they currently encode the same non-nullable shape as
the bug.

## Scope note for DBZ-3

DBZ-3 Area 2 is specifically about schema behavior across a restart. Proving crash-safety with
Avro schema attachment disabled (as the B0-2 harness does today) is a narrower claim than it
reads as - B0-3/B0-4 should make an explicit decision about whether to keep
`withAvroSchema=false` or fix this first, rather than silently inheriting the workaround.

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with source/schema/avro.go, especially avroExtractor.Extract, ExtractLogrepl, and extractType, then inspect the nullable columns in test/helper.go and TestTableAvroSchemaV1/V2. Determine how nullability should reach the extractor, using the issue's information_schema.columns/pg_attribute alternatives as the design starting point. Done means nullable supported columns produce matching nullable Avro schemas without breaking the existing extraction and integration tests.

Written by the indexing model from the issue text.

Assessment

Tech stack
go, postgresql
Domain
backend, databases
Issue type
Bug
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
45/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.