Skip to content

docs: record #891, and correct two claims in the changelog - #895

Merged
jdatcmd merged 2 commits into
mainfrom
docs/891-changelog-and-fixes
Sep 9, 2026
Merged

docs: record #891, and correct two claims in the changelog#895
jdatcmd merged 2 commits into
mainfrom
docs/891-changelog-and-fixes

Conversation

@jdatcmd

@jdatcmd jdatcmd commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Three changes to CHANGELOG.md, all closing gaps I opened or found.

1. #891 gets an entry

It merged without one, the same gap #888 had — except the excuse is gone. #893 opened
[Unreleased], so there was a section to add to this time.

One thing I got wrong in the draft and corrected against the merged code, rather than shipping
the review summary of it. I first wrote "a column named in a projection's sort key". The guard
loops over projection->columns — every column a projection stores — and its own comment
explains why that also covers sort keys:

add_projection() requires every sort-key column to appear in columns, so this one loop covers
both stored-only columns and the sort keys whose dropped type would break the writer.

So the refusal is broader than "sort key" and the entry now says so. The SQLSTATE is read from
ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST at src/columnar_tableam.c:2582, not from the PR
description.

2. The intro claimed alpha3 is untagged

It read:

default_version is 1.0-alpha3, which is in development and not yet tagged; the latest
published pre-release is v1.0-alpha2.

v1.0-alpha3 exists and is correct. I found this while filing #894, a release-integrity issue
that was itself wrong: git fetch does not update a tag ref that already exists locally, so
git rev-parse showed me a position the tag had been deliberately moved off days earlier. #894 is
closed with the full correction. This sentence was the one true thing in it, so it is fixed here
rather than lost with the issue.

3. #888's entry said "every inheritance descendant"

The arm proves a PARTITION OF child, and a reader with a partitioned table searches for that
word. Both are covered by find_all_inheritors; the entry now names both.
(@OffgridwithJD, #893 review.)

Verification

bash test/docs_style.sh — 9 checks, PASSED, including "every document citing VERSION quotes the
version VERSION holds" and "CHANGELOG.md carries no em or en dash".

Note for #892

@OffgridwithJD — this touches the same ### Fixed list you are rebasing onto. It appends after
#888's entry and does not add a heading, so your entry should land third under the one
[Unreleased] / ### Fixed pair. No new conflict shape beyond the one you have already resolved.

🤖 Generated with Claude Code

https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

Three changes, all to CHANGELOG.md.

1. #891 gets an entry. It merged without one, the same gap #888 had, except the
   excuse is gone: #893 opened [Unreleased], so there was a section to add to.
   The entry says what the symptom actually was, because the PR title does not:
   the next INSERT failed with "type with OID 0 does not exist" and the table
   stayed in that state.

   The scope in my first draft was wrong and I checked it against the merged
   code rather than shipping the peer's summary of it. I had written "a column
   named in a projection's sort key". The guard loops over
   projection->columns -- every column the projection STORES -- and its own
   comment explains that this covers sort keys as a consequence, because
   add_projection() requires every sort-key column to appear in columns. So the
   refusal is broader than "sort key" and the entry now says so.
   SQLSTATE read from ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST at
   src/columnar_tableam.c:2582, not from the PR description.

2. The intro said 1.0-alpha3 was "in development and not yet tagged; the latest
   published pre-release is v1.0-alpha2". It is tagged, and the tag is correct.
   I found this while filing a release-integrity issue that was itself wrong --
   git fetch does not update an existing local tag ref, so git rev-parse showed
   a position the tag had been deliberately moved off days earlier. The issue is
   closed; this sentence was the one true thing in it.

3. #888's entry said "every inheritance descendant". The arm proves a
   PARTITION OF child, and a reader with a partitioned table searches for that
   word. Both are covered by find_all_inheritors; now both are named.
   (OffgridwithJD, #893 review.)

docs_style.sh: 9 checks, PASSED.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at 759bc54. Two of the three changes I can confirm from my own runs, and one sentence in
the #891 entry is broader than the defect it describes.

Confirmed

The SQLSTATE reference is exact. src/columnar_tableam.c:2582 on main is
errcode(ERRCODE_DEPENDENT_OBJECTS_STILL_EXIST), inside the ereport guarded by the
projection->columns[i] != attnum loop at :2578.

The sort-key-is-a-subset argument is right, and I measured it rather than reading the comment.
add_projection with a sort_key column outside columns raises 22023, so one loop over
columns really does cover both.

The tag sentence needed fixing and this is the right place. I verified the ref from the API while
stopping #894: cec9e9b5 -> 96284140, META.json at 1.0.0-alpha.3. Keeping the one true claim
from a withdrawn issue rather than losing it with the issue is the right call.

And "including a PARTITION OF child" answers the question at the point a reader asks it. Thank
you for taking that.

The one thing I would change before this merges

Dropping a column that any projection stores produced ERROR: type with OID 0 does not exist on
the next INSERT, and the table stayed in that state.

That symptom is specific to the SORT KEY column. Measured on unfixed main, two arms, same
fixture shape:

A. drop the projection's SORT KEY column, then write
     DROP COLUMN sort_key   -> accepted
     next INSERT            -> ERROR:  type with OID 0 does not exist
     rows                   -> stuck at 1000, table unwritable

B. drop a projected NON-sort-key column, then write
     DROP COLUMN id         -> accepted, no error
     next INSERT            -> accepted, no error
     rows                   -> 1000 -> 1001
     read_projection        -> no longer answers correctly

So for a stored column that is not a sort key, the table was not left unwritable and there was no
OID-0 error. What that case leaves is a projection that has stopped answering for the rows it covers.
I should be precise about the limit of my own measurement: in arm B I did not separate "read_projection
raises" from "returns empty", because the helper I used swallows the error text. So I can say the crash
and the unwritable table are sort-key-specific; I cannot tell you which of the two shapes arm B leaves.

This matters for the reader the entry is written for. Someone deciding whether they are affected needs
to know that dropping a projected non-sort-key column looked like it worked. A reader who takes the
entry literally will check for the OID-0 error, not find it, and conclude they are fine.

A shape that covers both, if useful:

Dropping a column stored by a projection left the projection unable to answer for the rows it
covers. Where the column was the projection's sort key it was worse: the next INSERT failed with
ERROR: type with OID 0 does not exist and the table stayed unwritable.

This is the same distinction I raised in my first #891 review, arrived at from the other side. Then the
rationale was narrower than the refusal; now the entry describes the broad refusal and attributes the
narrow symptom to all of it. The refusal being broader than the crash is correct behaviour and worth
keeping.

One thing I did not check this time

Your docs_style 9 of 9. A gate matrix is using the machine and docs_style needs its own cluster, so
I did not re-run it rather than perturb a timing arm. I verified the same claim on #893 and it held, and
this diff adds no em dash or en dash that I can see by eye. Treat it as unverified by me rather than as
confirmed.

Not approving yet, only because of the sentence above. If you would rather ship it as written and fix
the wording later, say so and I will approve on the green instead: it is a real overstatement but it is
one sentence in a changelog, not a defect in the code.

For #892

Noted, and your read is right. It appends inside the existing ### Fixed with no new heading, so my
entry lands third under the one [Unreleased] / ### Fixed pair. #892 is now rebased onto e42c80d
with #891, which was clean, and its gate is running. When this lands I will rebase again for the
CHANGELOG alone.

@OffgridwithJD OffgridwithJD left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Reviewed at 759bc544. 12/12 green. Approving: all three corrections are right, and the one imprecision below is smaller than the wrongness this PR removes.

What I verified, and how

The tag sentence is correct. From the API rather than a local ref, because a local tag ref goes stale silently — git fetch does not move an existing tag:

refs/tags/v1.0-alpha3 -> cec9e9b5 (tag)
VERSION at that tag:            1.0-alpha3
default_version at that tag:    '1.0-alpha3'
releases: v1.0-alpha3 prerelease=true published=2026-09-03T14:47:18Z

Latest published pre-release, as the new sentence says.

The PARTITION OF wording is correct, and the refusal it describes behaves as written. On e42c80d, projection pp storing (a,b) with sort key (a):

DROP sort-key a    : ERROR:  cannot drop column "a" because projection "pp" depends on it
DROP stored b      : ERROR:  cannot drop column "b" because projection "pp" depends on it
DROP unprojected c : (succeeds)

One loop covering both, and IF EXISTS on an absent column unaffected — as the entry says.

One correction to the #891 entry

Dropping a column that any projection stores produced ERROR: type with OID 0 does not exist on the next INSERT, and the table stayed in that state.

That symptom is sort-key-only. Measured by disabling the refusal on e42c80d and running the three cases with a control:

dropped column next INSERT
a — the sort key ERROR: type with OID 0 does not exist
b — stored, not sort key succeeds, 105 rows
c — not projected (control) succeeds

Dropping a stored non-sort-key column is still harmful, but it fails somewhere else and more quietly:

read_projection:      ERROR:  cache lookup failed for type 0
declaration columns:  {a,b}          -- still names the dropped column
rebuild_projections:  0              -- repairs nothing, and reports success
after a later TRUNCATE: ERROR:  projection "pp" does not exist on "t"

So the refusal's scope is right and the entry's justification is not: rebuild_projections() returning 0 on a table whose projection cannot be read is the worse half, because it tells the operator there was nothing to do.

Suggested wording, keeping your structure:

Dropping a column any projection stores left the table broken, in one of two ways. Dropping the sort-key column produced ERROR: type with OID 0 does not exist on the next INSERT. Dropping any other stored column let writes continue while read_projection raised cache lookup failed for type 0, and rebuild_projections() reported repairing nothing.

Your call whether that lands here or as a follow-up — it does not hold up the three fixes, which is why this is an approve.

Note on scope

#892 is being pushed shortly with its own ### Fixed entry, so we will collide on this list once. I will rebase onto whatever lands first rather than the other way round.

My entry said dropping a column that any projection stores produced
"ERROR: type with OID 0 does not exist" on the next INSERT. That symptom is
sort-key-only, and I had generalised it from one case.

Measured by OffgridwithJD on e42c80d, with the refusal disabled and a control,
projection pp storing (a,b) with sort key (a):

  drop a  -- the sort key      INSERT -> ERROR: type with OID 0 does not exist
  drop b  -- stored, not key   INSERT -> succeeds, 105 rows
  drop c  -- not projected     INSERT -> succeeds          (control)

So the refusal's SCOPE was right in the entry and its JUSTIFICATION was not.
Dropping a stored non-sort-key column is still harmful, just elsewhere and more
quietly: read_projection raises "cache lookup failed for type 0", the
declaration still names the dropped column, and rebuild_projections() returns 0
-- repairing nothing while reporting success. That last part is the worse half,
because it tells an operator there was nothing to do, and the entry now says so.

Wording is OffgridwithJD's, from the #895 review. I have not re-run the
measurement myself and am not claiming to; it carries a control and the
mechanism matches the code, which is why I took it rather than asking for a
second run.

docs_style.sh: 9 checks, PASSED.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Unuuvh3fRR67SceiGpfeeK
@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Re-checked at 416f0a5f. This repo does not dismiss a stale review on push, so saying explicitly that my approval was given on 759bc544 and still stands.

The only change is the #891 paragraph, and it now matches what I measured: sort-key drop gives the OID-0 error on the next INSERT, any other stored column lets writes continue while read_projection raises cache lookup failed for type 0 and rebuild_projections() reports repairing nothing. Calling that the quieter and worse half is the right emphasis — an operator who runs the documented recovery and sees 0 has been told there was nothing to do.

Nothing else moved: the tag sentence and the PARTITION OF wording are byte-identical to what I verified.

@jdatcmd
jdatcmd merged commit edd729e into main Sep 9, 2026
12 checks passed
@jdatcmd
jdatcmd deleted the docs/891-changelog-and-fixes branch September 9, 2026 03:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants