tursodatabase / tursodatabase/libsql

Return i64 and f64 instead of &i64 and &f64 in Value

Open
#2,235 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C
Stars
17.2k
Forks
531
Avg merge
1h 12m
Merged PRs (30d)
1

Description

While going through the codebase, I noticed that Value::as_integer and Value::as_real return Option<&i64> and Option<&f64> respectively, rather than Option and Option.

Since i64 and f64 are both Copy and only 8 bytes (the same size as a reference on a 64-bit target), returning a reference offers no performance benefit and forces every caller to dereference. Is there a specific reason for this, or would a change to return by value be welcome?

For comparison, serde_json::Value::as_i64/as_f64 returns these by value.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Locate the definitions and callers of Value::as_integer and Value::as_real, then inspect nearby tests or API documentation for the current reference-returning behavior. Determine whether returning copied values is compatible with existing callers; done means the chosen return types are implemented consistently and relevant tests and callers pass.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
databases
Issue type
Feature
Difficulty
3/5
Estimated time
1-2 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
48/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.