| From: | Ewan Young <kdbase(dot)hack(at)gmail(dot)com> |
|---|---|
| To: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | [PATCH] seg: preserve the upper boundary's certainty indicator in seg_out() |
| Date: | 2026-06-11 07:03:54 |
| Message-ID: | CAON2xHPYeRRCEVAv8XfE18KsEsEHCiYcJ5fOsoxFuMEfpxF1=g@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Hackers,
While reviewing contrib/seg I noticed that seg_out() mishandles the
certainty indicator ('<', '>' or '~') on the upper boundary of an
interval, leading to wrong output and even silent data loss for valid
values.
For example, on current master:
regression=# SELECT '1.5 .. ~2.5'::seg;
seg
------------
1.5 .. 2.5 -- the '~' on the upper bound is dropped
regression=# SELECT '~6.5 .. 8.5'::seg;
seg
----------
~6.5 .. -- the upper bound 8.5 is lost entirely
The culprit is in seg_out() (contrib/seg/seg.c), where the code that
prints the upper boundary's indicator is:
if (seg->u_ext == '>' || seg->u_ext == '<' || seg->l_ext == '~')
p += sprintf(p, "%c", seg->u_ext);
The third test should examine u_ext, not l_ext -- it's a copy-and-paste
slip from the symmetric block that prints the lower boundary a few lines
above (which correctly tests l_ext three times).
This produces two distinct misbehaviours:
* A '~' on the upper boundary fails the (wrong) condition and is not
printed at all -> the indicator is dropped.
* When the lower boundary carries '~' but the upper boundary has no
indicator (u_ext == '\0'), the wrong test matches and
sprintf(p, "%c", seg->u_ext) writes a NUL byte into the output
buffer. PG_RETURN_CSTRING then stops at that NUL, truncating the
string and dropping the upper boundary value -> data loss.
Certainty indicators are documented to be preserved on output (they are
ignored by the operators, but kept as a comment), so this breaks the
input/output round-trip for the affected values.
The bug appears to date back to when seg was first added. It went
unnoticed because the existing regression tests only exercise certainty
indicators on single-point segs (e.g. '~6.5'), which are printed by a
different branch of seg_out() and never reach the buggy line.
The attached patch fixes the one-character typo and adds regression
tests that place each indicator on both boundaries of an interval, so
the upper-boundary case is now covered. make installcheck passes with
the fix and fails without it.
Regards,
Ewan Young
| Attachment | Content-Type | Size |
|---|---|---|
| 0001-seg-Fix-seg_out-to-preserve-the-upper-boundary-s-cer.patch | application/octet-stream | 4.1 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Masahiko Sawada | 2026-06-11 07:07:26 | Re: Support logical replication of DDLs, take2 |
| Previous Message | 반지현 | 2026-06-11 06:53:19 | Re: Return value of XLogInsertRecord() for XLOG_SWITCH record |