Skip to content

chore: check seed for CpcSketch and CountMinSketch#85

Open
tisonkun wants to merge 1 commit intomainfrom
check-seed
Open

chore: check seed for CpcSketch and CountMinSketch#85
tisonkun wants to merge 1 commit intomainfrom
check-seed

Conversation

@tisonkun
Copy link
Member

@tisonkun tisonkun commented Feb 8, 2026

This follows #84 (comment).

I have two questions:

  1. Shall we check the seed when creating sketches? Currently, we check only when serde, thus if the sketch never serde, we don't have this guard. Not sure if check earilier helps.
  2. Since the serialized seed_hash occurs in the middle of the final result, I'm curious about the format of earlier serialized sketches. Do they have a placeholder 2-byte buffer that is fulfilled in later version? If we panic always when the computed seed = 0, then all those old snapshots cannot be read.

cc @ZENOTME @leerho

Signed-off-by: tison <wander4096@gmail.com>
@ZENOTME
Copy link
Contributor

ZENOTME commented Feb 8, 2026

  1. Shall we check the seed when creating sketches? Currently, we check only when serde, thus if the sketch never serde, we don't have this guard. Not sure if check earilier helps.

I think we should check the seed when creating sketches. I think it's not convenient to change the seek after using the sketch, so it's better to guarantee that the seed hash is valid at begining.

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