Skip to content

Add compact_chunk function#9957

Open
antekresic wants to merge 1 commit into
mainfrom
antekresic/compaction_poc
Open

Add compact_chunk function#9957
antekresic wants to merge 1 commit into
mainfrom
antekresic/compaction_poc

Conversation

@antekresic

Copy link
Copy Markdown
Member

This new function will compact the chunk by looking for overlapping batches and combining them together in order to produce globally ordered chunks. This is change is a first step towards supporting direct compress in production workloads.

@antekresic antekresic self-assigned this Jun 3, 2026
@antekresic antekresic added the enhancement An enhancement to an existing feature for functionality label Jun 3, 2026
@codecov

codecov Bot commented Jun 3, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.24460% with 16 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
tsl/src/compression/recompress.c 94.11% 3 Missing and 13 partials ⚠️

📢 Thoughts on this report? Let us know!

@antekresic antekresic force-pushed the antekresic/compaction_poc branch from ff03edc to 2dce6bf Compare June 3, 2026 11:20
@antekresic antekresic marked this pull request as ready for review June 3, 2026 13:42
@github-actions github-actions Bot requested review from kpan2034 and svenklemm June 3, 2026 13:42
@github-actions

github-actions Bot commented Jun 3, 2026

Copy link
Copy Markdown

@svenklemm, @kpan2034: please review this pull request.

Powered by pull-review

Comment thread tsl/src/compression/recompress.c
Comment thread tsl/src/compression/recompress.c
Comment thread tsl/src/compression/recompress.c Outdated
@antekresic antekresic force-pushed the antekresic/compaction_poc branch from 2dce6bf to 3a9fb1a Compare June 16, 2026 10:36

@svenklemm svenklemm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@antekresic antekresic force-pushed the antekresic/compaction_poc branch from 3a9fb1a to 1d729b9 Compare June 19, 2026 08:08
This new function will compact the chunk by looking for
overlapping batches and combining them together in order
to produce globally ordered chunks. This is change is
a first step towards supporting direct compress in
production workloads.
08:20 > 08:11 → OVERLAP → merge
```

### 6. Mixed-null batch overlaps a neighbor

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do we still have special handling for nulls? I thought the firstlast index just handles this transparently, i.e. the comparison gives total order that includes the nulls, and we just apply the comparison regardless of the values.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Only special handling that is in place is that nulls get put into a separate batch... I guess that's not necessary anymore and is more of an artifact of previous version.

Do you think that's fine or should we handle it without separating the NULL batch.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'd remove it because it just extra logic for no particular reason, looks confusing.

* using each column's NULLS FIRST/LAST setting.
*/
static bool
batches_overlap_firstlast(RecompressContext *recompress_ctx, Datum *prev_last,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is basically comparison of tuples on particular columns. Would it be more convenient to reuse the standard SortSupport and ApplySortComparator Postgres functions?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Can take a look if it makes our lives easier.

@akuzm akuzm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks much more straighforward with the new sparse index type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement An enhancement to an existing feature for functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants