| From: | Joao Rito <joaovictor(dot)rito(at)gmail(dot)com> |
|---|---|
| To: | Mingjie Gao <mingjie(dot)gmj(at)gmail(dot)com> |
| Cc: | vignesh C <vignesh21(at)gmail(dot)com>, Peter Smith <smithpb2250(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org |
| Subject: | Re: [PATCH v1] Fix initial sync of zero-column published tables |
| Date: | 2026-11-12 18:56:22 |
| Message-ID: | 20261112185622.73614-1-joaovictor.rito@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Mingjie,
Thank you for the patch. I tried v3 and it looks good to me.
What I did:
- Applied v3 to current master (debfb3c8a8). It applies cleanly and builds
without warnings (--enable-cassert --enable-tap-tests).
- make check passes (243 tests), also contrib/file_fdw. The whole
src/test/subscription TAP suite passes with the patch (39 files, 618
tests; 012_collation was skipped because I built without ICU).
- I applied only the 008_diff_schema.pl hunk to unpatched master. The test
fails there: the initial sync of tab_nocols never completes (the table
stays in state 'd'). So the new test does catch the bug.
- I reproduced the problem by hand on unpatched master with a zero-column
publisher table (two rows) and a subscriber table with a local default
column:
- binary = false: the local column gets '' instead of its DEFAULT.
- binary = true: "ERROR: row field count is 0, expected 1", and the sync
never finishes.
- If the extra subscriber column is not text (a stored generated plus
identity column, an integer partition key, or an integer NOT NULL column
with a default), text mode also fails with "invalid input syntax for
type integer: """, and the sync keeps retrying.
With the patch all of these cases finish (state 'r') and the rows load,
with the local defaults where they are defined, in both text and binary
mode. This included a stored generated column plus an identity column, a
partitioned subscriber table, and a publisher table whose only column had
been dropped. A normal one-column publisher table with an extra subscriber
column behaves the same before and after.
The change to BeginCopyFrom() looks fine to me. Keeping it as a wrapper
around BeginCopyFromExtended() leaves the other callers untouched, and
passing true from copy_table() is the only place that needs the new
behavior.
I have not tested the back branches, a publication column list that selects
no columns, or a meson build, so I can not say anything about those.
Thanks again,
Joao Rito
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | Raja Sai pranav | 2026-11-12 18:46:40 | Re: Re: Assertion failure in WAL summarizer |