Skip to content

Breaking change: Make Rows and Row API more consistent.#227

Open
coco-speed wants to merge 1 commit intomainfrom
fork-1623-ignatz/align_row_and_rows
Open

Breaking change: Make Rows and Row API more consistent.#227
coco-speed wants to merge 1 commit intomainfrom
fork-1623-ignatz/align_row_and_rows

Conversation

@coco-speed
Copy link
Collaborator

This pull request was created automatically by CodSpeed to track performance changes of the pull request tursodatabase/libsql#1623.

The original branch is fork-1623-ignatz/align_row_and_rows

@codspeed-hq
Copy link

codspeed-hq bot commented Aug 1, 2024

CodSpeed Performance Report

Merging #227 will not alter performance

Comparing fork-1623-ignatz/align_row_and_rows (f510359) with main (a9359d7)

Summary

✅ 4 untouched benchmarks

A few notes: I went the path of least resistance also assuming it would
break fewer folks, i.e. make Row more like Rows and thus usize -> i32.

Arguably, an unsigned type might be more appropriate both for length and
indexes. I understand that the i32 stems from the sqlite bindings, which
returns/accepts c_ints. Yet, Statement and Row made the jump to an
unsigned, probably drawing the same conclusion, whereas Rows preserved
its proximity to the c-bindings. This is probably an artifact?

Also going on a limb, mapping c_int -> i32 is already a non-portable
choice, with precision for c_int being platform dependent.
@coco-speed coco-speed force-pushed the fork-1623-ignatz/align_row_and_rows branch from 67f2761 to f510359 Compare August 1, 2024 11:45
@coco-speed coco-speed force-pushed the main branch 8 times, most recently from e1cdddc to f972697 Compare August 6, 2024 11:30
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