Skip to content

adding compaction from lib - #41

Open
rubend056 wants to merge 6 commits into
njaard:masterfrom
rubend056:master
Open

adding compaction from lib#41
rubend056 wants to merge 6 commits into
njaard:masterfrom
rubend056:master

Conversation

@rubend056

Copy link
Copy Markdown

Pull request for issue #38

@njaard

njaard commented Jun 5, 2024

Copy link
Copy Markdown
Owner

Hi @rubend056 , thanks for your contribution.

Originally I wanted didn't want to commit to a public API for this, but obviously you have a use for it, so I'm happy to merge.

But, can you take a look at the CI failures?

- f32::NAN / f64::NAN instead of deprecated ::std::f32::NAN paths
- explicit Option<Segment<'_>> in segment_reader to silence
  hidden-lifetime lints
- drop leftover commented println! in compact()
- cargo fmt
- compact(): propagate add_record_raw failures as io::Error instead of
  panicking; document the non-blocking .compact lock; drop a dead
  condition that cloned every transaction path per call
- formatted::add_from_stream_with_fmt: cache the parsed row format
  instead of re-parsing (and re-boxing) it for every input line — this
  is also the --gegnum compaction ingest path
- merge: comparator is Arc<dyn Fn> rather than Arc<Box<dyn Fn>>,
  removing a pointer chase from every per-record comparison
- changelog: Unreleased section
- io::Error::other over ErrorKind::Other construction (io_other_error)
- str::len over as_bytes().len() (needless_as_bytes)
- .any(f) over .map(f).any(identity) in RecordBuilder impls
- let-match over late initialization in compressed_bytes
- indent doc-comment list continuations (doc_lazy_continuation)
- reap the --gegnum child on the failure path too (zombie_processes)

cargo clippy --all-features is now warning-free apart from a
future-incompat note from the static_init dependency.
@rubend056

Copy link
Copy Markdown
Author

Maybe a few years too late :P LOL but I did a few tidy up changes and tiny fixes.

CI should pass cleanly, zero warnings, clippy clean :)

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