Skip to content

Conversation

@amigin
Copy link
Contributor

@amigin amigin commented Jan 13, 2026

Note

Major refactor and upgrades across crates with breaking API changes and new server components.

  • Introduces Timestamp type; MyNoSqlEntity::get_time_stamp now returns Timestamp and MyNoSqlEntitySerializer::deserialize_entity returns Result (breaking)
  • Adds LAZY_DESERIALIZATION flag on entities/enums; macros updated (supports with_expires) and safer field generation; enum/table macros now return Result on deserialize
  • Migrates to newer my-json API: switches to JsonValueWriter and JsonFirstLineIterator/JsonArrayIterator; many internals now write to String
  • Refactors data-writer: new FlUrlFactory, retries via FlUrl, optional SSH feature, partition-keys fetch API, and background ping pool; improved error handling
  • Adds new my-no-sql-server-core crate with DbTable wrapper and snapshot models for tables/partitions/rows
  • Updates deps (rust-extensions, my-json, flurl, my-tcp-sockets, my-logger) and bumps versions to 0.4.x
  • CI: runs default tests, master-node feature tests, and all-features build; adds design patterns doc for entities

Written by Cursor Bugbot for commit 2921aff. This will update automatically on new commits. Configure here.

Copy link

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

This PR is being reviewed by Cursor Bugbot

Details

You are on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle.

To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.

println!("Can not parse timestamp: {}", s);
}

Ok(Timestamp(datetime.unwrap().unix_microseconds))
Copy link

Choose a reason for hiding this comment

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

Timestamp deserialization panics on invalid input

High Severity

The Timestamp deserialize implementation has a panic when parsing fails. When DateTimeAsMicroseconds::from_str() returns None, the code prints an error message but then immediately calls datetime.unwrap() on the None value, causing a panic. This will crash the application when deserializing entities with malformed timestamp strings.

Fix in Cursor Fix in Web

) -> FlUrlBody {
if entities.len() == 0 {
return Some(vec![b'[', b']']);
FlUrlBody::Json(vec![b'[', b']']);
Copy link

Choose a reason for hiding this comment

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

Missing return statement causes dead code path

Low Severity

In serialize_entities_to_body, when entities.len() == 0, the code creates FlUrlBody::Json(vec![b'[', b']']) but doesn't return it. The expression is evaluated and immediately discarded, then the function falls through to build an empty JSON array anyway. This appears to be accidental - a return keyword is missing.

Fix in Cursor Fix in Web

} else {
self.index[index].items.push(item.to_owned());
false
}
Copy link

Choose a reason for hiding this comment

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

Expiration index count wrong when adding to bucket

Medium Severity

In the add method, when an item is successfully added to an existing expiration bucket (line 69), the function returns false (line 70). Since amount is only incremented when the return value is true (line 83-84), the amount counter becomes incorrect whenever items are added to existing buckets rather than creating new ones. This causes the len() method to return inaccurate counts.

Fix in Cursor Fix in Web

.get(row_to_delete.row_key.as_str())
.unwrap()
.clone(),
);
Copy link

Choose a reason for hiding this comment

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

Panic when deleting rows with callbacks enabled

High Severity

In delete_rows, when callbacks are enabled, the code first removes the row from the partition via partition.remove() on line 155, then immediately tries to retrieve the same row via partition.get() on line 167. Since the row was just removed, get() returns None and the subsequent .unwrap() causes a panic. The removed row should be captured from the remove() return value instead of trying to fetch it again.

Fix in Cursor Fix in Web

InitPartitionResult {
partition_before: entities.get(partition_key).unwrap(),
partition_now: before_partition,
}
Copy link

Choose a reason for hiding this comment

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

Swapped before/now field assignments in init_partition result

Medium Severity

In init_partition, the InitPartitionResult field assignments are swapped. The partition_before field is assigned entities.get(partition_key) which returns the newly inserted partition, while partition_now is assigned before_partition which contains the old removed data. The semantic meaning of these field names is inverted - callers expecting partition_before to contain the old state and partition_now to contain the current state will get the opposite.

Fix in Cursor Fix in Web

@amigin amigin closed this Jan 13, 2026
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.

3 participants