Skip to content

test: create_rollup's last refusals and rebase_rollup's, in both twins (#1341) - #1359

Merged
jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-rollup-refusals
Oct 14, 2026
Merged

jdatcmd merged 2 commits into
commandprompt:mainfrom
OffgridwithJD:test/1341-rollup-refusals

Conversation

@OffgridwithJD

Copy link
Copy Markdown
Collaborator

Part of #1341: create_rollup's remaining refusals and rebase_rollup's, from the medium and low items. Every arm is in both twins, next to the existing refusal arms. Each is red when the check it names is deleted from pgcolumnar--1.0-alpha5.sql, and main survives each deletion.

arm mutation main this branch
a ts_column that is not timestamptz is refused the timestamptz check removed sh 72/72, py passed "got [ok] want [22023]", and "no refused rollup left a table behind: got [1]"
a group_by column the source does not have is refused the group_by column check removed green "got [42601] want [22023]": the CREATE TABLE breaks instead
a sum over a column that cannot be folded is refused the cannot-be-folded check removed green "got [42601] want [22023]"
rebase_rollup of a table that is not a rollup is refused (42704) its IF NOT FOUND removed green "got [42601] want [42704]"

The timestamp case is the one that matters most. A timestamp (no zone) source column would be bucketed in whatever zone the refresh happens to run in, so the refusal is the documented behaviour. With the check gone, the create succeeds, which is why the existing "no table left behind" arm also gains this mutation in the ledger.

Evidence

  • Clean: rollup 76/76 and test_rollup.py passed on each of PG 15–19.
  • Ledger: four new dated rows; 2336 → 2340 rows, census unchanged at 1803, restated at 8af86e8.
  • Gate: rc=0.
  • Database-free set: 429 passed.

Overlap with #1357: both touch rollup.sh and test_rollup.py in different places, and both move the budget, so whichever lands second restates.

🤖 Generated with Claude Code

@jdatcmd jdatcmd 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.

Approving the content at 8ed2b65. On PG17 I reproduced the timestamptz claim with create_rollup's check disabled:

  • main: rollup passes 72/72.
  • this head: "a ts_column that is not timestamptz is refused: got [ok] want [22023]" and "and no refused rollup left a table behind: got [1] want [0]". That second arm gaining this mutation is right, as you say.

Before merge: CI hadn't reported on this head when I looked, and main has moved to 1933577 (2343 / 1796). With your four dated rows I expect 2347 / 1796. It also overlaps #1357 in rollup.sh and test_rollup.py. I suggest #1357 lands first, then this one rebases onto it.

jdatcmd and others added 2 commits October 14, 2026 01:52
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

@jdatcmd, rebased onto 8c61dff (#1357 merged). The new head is 7af80c9, replacing 8ed2b65. Ledger: 2353 / 1798 (2349 + 4 dated).

I'll ping on green CI.

@OffgridwithJD
OffgridwithJD force-pushed the test/1341-rollup-refusals branch from 8ed2b65 to 7af80c9 Compare October 14, 2026 01:58
@OffgridwithJD

Copy link
Copy Markdown
Collaborator Author

@jdatcmd, CI is green on 7af80c9 (15/15). With my run of the merged rollup files on 15–19 (82/82, posted above), it's ready for your merge. #1361 is next, and I'll rebase it as soon as this lands.

@jdatcmd jdatcmd 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.

Re-approving 7af80c9.

  • Ledger: a recount gives 2353 / 1798, which agrees with the budget. No duplicate keys, no dated premise: row.
  • Patch: the rollup changes are identical to 8ed2b65, the head I reviewed and reproduced.
  • Database-free set: 429 passed.

Merging.

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