-
Notifications
You must be signed in to change notification settings - Fork 76
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
chore(weave): table row response includes absolute row indices #3590
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
a1f7dc6
to
e5c77bb
Compare
@@ -36,21 +36,22 @@ def make_natural_sort_table_query( | |||
row_digests_selection = f"arraySlice({row_digests_selection}, 1 + {{{pb.add_param(offset)}: Int64}}, {{{pb.add_param(limit)}: Int64}})" | |||
|
|||
query = f""" | |||
SELECT DISTINCT tr.digest, tr.val_dump, t.row_order | |||
SELECT DISTINCT tr.digest, tr.val_dump, t.original_index + {{{pb.add_param(offset or 0)}: Int64}} - 1 as original_index |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adding the offset here is needed to get the absolute index instead of the index relative to the offset.
e5c77bb
to
8ac6e07
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please author a test suite to verify correctness
907697b
to
c3dc8ec
Compare
@tssweeney I've updated the exiting suite to test the correctness of the returned indices, which also prompted me to add the index to the response from the sqlite implementation |
c3dc8ec
to
99dbe7b
Compare
Description
Return absolute index of each row when serving rows from a table. This enables more powerful UIs for editing datasets.