Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[SQLite] Add .columnNames property on Cursor #911

Merged
merged 4 commits into from
Jul 27, 2023
Merged

Conversation

geelen
Copy link
Contributor

@geelen geelen commented Jul 25, 2023

(Replaces #696 as I renamed the remote branch name and the other PR got closed)

This PR adds a .columnNames property that can be called on a Cursor before iterating over the result:

// Given the following data:
sql.exec(`CREATE TABLE abc (a INT, b INT, c INT);`)
sql.exec(`CREATE TABLE cde (c INT, d INT, e INT);`)
sql.exec(`INSERT INTO abc VALUES (1,2,3),(4,5,6);`)
sql.exec(`INSERT INTO cde VALUES (7,8,9),(1,2,3);`)

const iterator = sql.prepare(`SELECT * FROM abc, cde`)();
console.log(iterator.columnNames)
// ["a","b","c","c","d","e"]
console.log(Array.from(iterator.raw()))
// [
//   [1, 2, 3, 7, 8, 9],
//   [1, 2, 3, 1, 2, 3],
//   [4, 5, 6, 7, 8, 9],
//   [4, 5, 6, 1, 2, 3],
// ]

This helps us get around the issue where the default iterator swallows data when column names collide:

const iterator = sql.prepare(`SELECT * FROM abc, cde`)();
console.log(Array.from(iterator))
// [
//   { a: 1, b: 2, c: 7, d: 8, e: 9 },
//   { a: 1, b: 2, c: 1, d: 2, e: 3 },
//   { a: 4, b: 5, c: 7, d: 8, e: 9 },
//   { a: 4, b: 5, c: 1, d: 2, e: 3 },
// ]

As a separate PR, we could make the default iterator error if duplicate columns are detected, but I want to move D1 entirely over to using .all() internally first.

@geelen geelen requested review from kentonv and jasnell July 25, 2023 04:01
Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM but let's have @kentonv sign off also.

src/workerd/api/sql.c++ Outdated Show resolved Hide resolved
@geelen geelen force-pushed the glen/sqlite-column-names branch from b79d386 to 4cbb82d Compare July 26, 2023 03:46
src/workerd/api/sql.c++ Outdated Show resolved Hide resolved
@geelen geelen force-pushed the glen/sqlite-column-names branch 2 times, most recently from b59cdae to 684ae74 Compare July 27, 2023 06:34
@geelen geelen force-pushed the glen/sqlite-column-names branch from 684ae74 to 8848146 Compare July 27, 2023 21:49
@geelen geelen merged commit 9af52dc into main Jul 27, 2023
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