> I found a crash in the logical replication sequence synchronization worker > when the publisher returns NULL sequence data for a sequence after at least > one sequence in the same sync batch has already been processed.
Good catch. I confirmed the HEAD can crash with your added test.
Thanks for checking and confirming the crash.
> The attached patch clears the output Relation pointer at the start of > get_and_validate_seq_info() and clears the local pointer in copy_sequences() > after closing it. That keeps early returns from reusing a relation from a > previous row.
To confirm; can't we declare the sequence_rel in the inner-loop? My first impression was the bug caused by the wrong lifetime. Are there any other thoughts around here?
I agree that declaring sequence_rel in the tuple-processing loop is cleaner. The relation belongs only to the current publisher result row, so limiting the variable's lifetime makes the intended ownership clearer and prevents any value from carrying over to the next row.
I have kept the initialization of the output argument in get_and_validate_seq_info(), so every return path leaves it in a defined state. In v2, the caller-side pointer is declared inside the inner loop, and the explicit reset after table_close() is no longer needed.