Skip to content

Conversation

@zxq82lm
Copy link

@zxq82lm zxq82lm commented Nov 16, 2025

PR objective

Address the issue #147.

Testing

Unit test:

cargo test numeric_id_expr

@zxq82lm zxq82lm changed the title feat/id-from-raw-int Allow constructing Id<T> from raw integers Nov 16, 2025
@zxq82lm zxq82lm marked this pull request as ready for review November 16, 2025 02:41
@carllerche
Copy link
Member

@claude /review-pr

@claude
Copy link

claude bot commented Nov 17, 2025

Claude encountered an error —— View job

Failed with exit code 128

I'll analyze this and get back to you.

Copy link
Member

@carllerche carllerche left a comment

Choose a reason for hiding this comment

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

Thanks for the PR. Just FYI, I plan on completely getting rid of the ID type soon. I am OK merging a PR to make it easier to work w/ it in the mean time. Is there a reason you are converting an Int ID -> Value::String?

pub fn to_primitive(&self) -> stmt::Value {
match &self.repr {
Repr::Int(_) => todo!(),
Repr::Int(id) => stmt::Value::String(id.to_string()),
Copy link
Member

Choose a reason for hiding this comment

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

Is there a reason you are converting an Int ID to a string?

Copy link
Author

Choose a reason for hiding this comment

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

I’m doing the Int-backed Id → Value::String conversion because the current pipeline maps Type::Id to a string column (stmt_ty_to_table does Type::Id(_) => Type::String), and the drivers already serialize IDs as text. If you’d prefer a different representation I’m happy to adjust.

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