issue_comments
14 rows where issue = 1573424830
This data as json, CSV (advanced)
Suggested facets: created_at (date), updated_at (date)
| id ▼ | html_url | issue_url | node_id | user | created_at | updated_at | author_association | body | reactions | issue | performed_via_github_app |
|---|---|---|---|---|---|---|---|---|---|---|---|
| 1419916684 | https://github.com/simonw/datasette/issues/2019#issuecomment-1419916684 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UojWM | simonw 9599 | 2023-02-06T23:09:51Z | 2023-02-06T23:10:13Z | OWNER | The inputs and outputs for this are pretty complex. Inputs: - `?_next=` from the query string - `is_view` - is this for a table or view? If it's a view it uses offset/limit pagination - which could actually work for arbitrary queries too. Also views could have keyset pagination if they are known to be sorted by a particular column. - `sort` and `sort_desc` reflecting the current sort order - `use_rowid` for if the table is a rowid table with no primary key of its own - `pks` - the primary keys for the table - `params` - the current set of parameters, I think used just to count their length so new params can be added as `p5` etc without collisions. This could be handled with a `s0`, `s1` etc naming convention instead. Outputs: - `where_clauses` - a list of where clauses to add to the query - `params` - additional parameters to use with the query due to the new where clauses - `order_by` - the order by clause | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1419917661 | https://github.com/simonw/datasette/issues/2019#issuecomment-1419917661 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5Uojld | simonw 9599 | 2023-02-06T23:10:51Z | 2023-02-06T23:10:51Z | OWNER | I should turn `sort` and `sort_desc` into an object representing the sort order earlier in the code. I should also create something that bundles together `pks` and `use_rowid` and maybe `is_view` as well. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1419921228 | https://github.com/simonw/datasette/issues/2019#issuecomment-1419921228 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UokdM | simonw 9599 | 2023-02-06T23:14:15Z | 2023-02-06T23:14:15Z | OWNER | Crucial utility function: https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/utils/__init__.py#L137-L160 | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1419928455 | https://github.com/simonw/datasette/issues/2019#issuecomment-1419928455 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UomOH | simonw 9599 | 2023-02-06T23:21:50Z | 2023-02-06T23:21:50Z | OWNER | Found more logic relating to this: https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/views/table.py#L684-L732 | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1419953256 | https://github.com/simonw/datasette/issues/2019#issuecomment-1419953256 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UosRo | simonw 9599 | 2023-02-06T23:42:56Z | 2023-02-06T23:43:10Z | OWNER | Relevant issue: - https://github.com/simonw/datasette/issues/1773 Explains this comment: https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/views/table.py#L697 | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1420088670 | https://github.com/simonw/datasette/issues/2019#issuecomment-1420088670 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UpNVe | simonw 9599 | 2023-02-07T02:14:35Z | 2023-02-07T02:14:35Z | OWNER | Maybe the correct level of abstraction here is that pagination is something that happens to a SQL query that is defined as SQL and params, without an order by or limit. That's then wrapped in a sub-select and those things are added to it, plus the necessary `where` clauses depending on the page. Need to check that the query plan for pagination of a subquery isn't slower than the plan for pagination as it works today. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1420094396 | https://github.com/simonw/datasette/issues/2019#issuecomment-1420094396 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UpOu8 | simonw 9599 | 2023-02-07T02:18:11Z | 2023-02-07T02:19:16Z | OWNER | For the SQL underlying this page (the second page in that compound primary key paginated sequence): https://latest.datasette.io/fixtures/compound_three_primary_keys?_next=a%2Cd%2Cv The explain for the default query: https://latest.datasette.io/fixtures?sql=explain+select%0D%0A++pk1%2C%0D%0A++pk2%2C%0D%0A++pk3%2C%0D%0A++content%0D%0Afrom%0D%0A++compound_three_primary_keys%0D%0Awhere%0D%0A++%28%0D%0A++++%28pk1+%3E+%3Ap0%29%0D%0A++++or+%28%0D%0A++++++pk1+%3D+%3Ap0%0D%0A++++++and+pk2+%3E+%3Ap1%0D%0A++++%29%0D%0A++++or+%28%0D%0A++++++pk1+%3D+%3Ap0%0D%0A++++++and+pk2+%3D+%3Ap1%0D%0A++++++and+pk3+%3E+%3Ap2%0D%0A++++%29%0D%0A++%29%0D%0Aorder+by%0D%0A++pk1%2C%0D%0A++pk2%2C%0D%0A++pk3%0D%0Alimit%0D%0A++101&p0=a&p1=d&p2=v The explain for that query rewritten as this: ```sql explain select * from ( select pk1, pk2, pk3, content from compound_three_primary_keys ) where ( (pk1 > :p0) or ( pk1 = :p0 and pk2 > :p1 ) or ( pk1 = :p0 and pk2 = :p1 and pk3 > :p2 ) ) order by pk1, pk2, pk3 limit 101 ``` https://latest.datasette.io/fixtures?sql=explain+select+*+from+%28select+%0D%0A++pk1%2C%0D%0A++pk2%2C%0D%0A++pk3%2C%0D%0A++content%0D%0Afrom%0D%0A++compound_three_primary_keys%0D%0A%29%0D%0A++where%0D%0A++%28%0D%0A++++%28pk1+%3E+%3Ap0%29%0D%0A++++or+%28%0D%0A++++++pk1+%3D+%3Ap0%0D%0A++++++and+pk2+%3E+%3Ap1%0D%0A++++%29%0D%0A++++or+%28%0D%0A++++++pk1+%3D+%3Ap0%0D%0A++++++and+pk2+%3D+%3Ap1%0D%0A++++++and+pk3+%3E+%3Ap2%0D%0A++++%29%0D%0A++%29%0D%0Aorder+by%0D%0A++pk1%2C%0D%0A++pk2%2C%0D%0A++pk3%0D%0Alimit%0D%0A++101&p0=a&p1=d&p2=v Both explains have 31 steps and look pretty much identical. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1420101175 | https://github.com/simonw/datasette/issues/2019#issuecomment-1420101175 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UpQY3 | simonw 9599 | 2023-02-07T02:22:11Z | 2023-02-07T02:22:11Z | OWNER | A more complex example: https://latest.datasette.io/fixtures/sortable?_next=0~2E2650566289400591%2Ca%2Cu&_sort=sortable_with_nulls_2 SQL: ```sql select pk1, pk2, content, sortable, sortable_with_nulls, sortable_with_nulls_2, text from sortable where (sortable_with_nulls_2 > :p2 or (sortable_with_nulls_2 = :p2 and ((pk1 > :p0) or (pk1 = :p0 and pk2 > :p1)))) order by sortable_with_nulls_2, pk1, pk2 limit 101 ``` https://latest.datasette.io/fixtures?sql=select+pk1%2C+pk2%2C+content%2C+sortable%2C+sortable_with_nulls%2C+sortable_with_nulls_2%2C+text+from+sortable+where+%28sortable_with_nulls_2+%3E+%3Ap2+or+%28sortable_with_nulls_2+%3D+%3Ap2+and+%28%28pk1+%3E+%3Ap0%29%0A++or%0A%28pk1+%3D+%3Ap0+and+pk2+%3E+%3Ap1%29%29%29%29+order+by+sortable_with_nulls_2%2C+pk1%2C+pk2+limit+101&p0=a&p1=u&p2=0.2650566289400591 Here's the explain: 49 steps long https://latest.datasette.io/fixtures?sql=explain+select+pk1%2C+pk2%2C+content%2C+sortable%2C+sortable_with_nulls%2C+sortable_with_nulls_2%2C+text+from+sortable+where+%28sortable_with_nulls_2+%3E+%3Ap2+or+%28sortable_with_nulls_2+%3D+%3Ap2+and+%28%28pk1+%3E+%3Ap0%29%0D%0A++or%0D%0A%28pk1+%3D+%3Ap0+and+pk2+%3E+%3Ap1%29%29%29%29+order+by+sortable_with_nulls_2%2C+pk1%2C+pk2+limit+101&p2=0.2650566289400591&p0=a&p1=u Rewritten with a subselect: ```sql select * from ( select pk1, pk2, content, sortable, sortable_with_nulls, sortable_with_nulls_2, text from sortable ) where (sortable_with_nulls_2 > :p2 or (sortable_with_nulls_2 = :p2 and ((pk1 > :p0) or (pk1 = :p0 and pk2 > :p1)))) order by sortable_with_nulls_2, pk1, pk2 limit 101 ``` https://latest.datasette.io/fixtures?sql=select+*+from+(%0D%0A++select+pk1%2C+pk2%2C+content%2C+sortable%2C+sortable_with_nulls%2C+sortable_with_nulls_2%2C+text+from+sortable%0D%0A)%0D%0Awhere+(sortable_with_nulls_2+%3E+%3Ap2+or+(sortable_with_nulls_2+%3D+%3Ap2+and+((pk1+%3E+%3Ap0)%0D%0A++or%0D%0A(pk1+%3D+%3Ap0+and+pk2+%3E+%3Ap1))))+order+by+sortable_with_nulls_2%2C+pk1%2C+pk2+limit+101&p2=0.2650566289400591&p0=a&p1=u … | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1420104254 | https://github.com/simonw/datasette/issues/2019#issuecomment-1420104254 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UpRI- | simonw 9599 | 2023-02-07T02:24:46Z | 2023-02-07T02:24:46Z | OWNER | Even more complicated: https://latest.datasette.io/fixtures/sortable?sortable_with_nulls__notnull=1&_next=0~2E692704598586882%2Ce%2Cr&_sort=sortable_with_nulls_2 The rewritten SQL for that is: ```sql select * from (select pk1, pk2, content, sortable, sortable_with_nulls, sortable_with_nulls_2, text from sortable where "sortable_with_nulls" is not null) where (sortable_with_nulls_2 > :p2 or (sortable_with_nulls_2 = :p2 and ((pk1 > :p0) or (pk1 = :p0 and pk2 > :p1)))) order by sortable_with_nulls_2, pk1, pk2 limit 101 ``` And it still has the same number of explain steps as the current SQL witohut the subselect. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1420106315 | https://github.com/simonw/datasette/issues/2019#issuecomment-1420106315 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UpRpL | simonw 9599 | 2023-02-07T02:28:03Z | 2023-02-07T02:28:36Z | OWNER | So I think I can write an abstraction that applies keyset pagination to ANY arbitrary SQL query provided it is given the query, the existing params (so it can pick names for the new params that won't overlap with them), the desired sort order, any existing `_next` token AND the columns that should be used to tie-break any duplicates. Those tie breakers will be either the primary key(s) or `rowid` if none are provided. What about the case of SQL views, where offset/limit should be used instead? I'm inclined to have that as a separate pagination abstraction entirely, with the calling code deciding which pagination helper to use based on if keyset pagination makes sense or not. Might be easier to design a class structure for this starting with `OffsetPaginator`, then using that to inform the design of `KeysetPaginator`. Might put these in `datasette.utils.pagination` to start off with, then maybe extract them out to `sqlite-utils` later once they've proven themselves. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1420109153 | https://github.com/simonw/datasette/issues/2019#issuecomment-1420109153 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5UpSVh | simonw 9599 | 2023-02-07T02:32:36Z | 2023-02-07T02:32:36Z | OWNER | Doing this as a class makes sense to me. There are a few steps: - Instantiate the class with the information it needs, which includes sort order, page size, tiebreaker columns and SQL query and parameters - Generate the new SQL query that will actually be executed - maybe this takes the optional `_next` parameter? This returns the SQL and params that should be executed, where the SQL now includes pagination logic plus order by and limit - The calling code then gets to execute the SQL query to fetch the rows - Last step: those rows are passed to a paginator method which returns `(rows, next)` - where `rows` is the rows truncated to the correct length (really just with the last one cut off if it's too long for the length) and `next` is either `None` or a token, depending on if there should be a next page. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1421274434 | https://github.com/simonw/datasette/issues/2019#issuecomment-1421274434 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5Utu1C | simonw 9599 | 2023-02-07T18:42:42Z | 2023-02-07T18:42:42Z | OWNER | I'm going to build completely separate tests for this in `test_pagination.py`. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1421600789 | https://github.com/simonw/datasette/issues/2019#issuecomment-1421600789 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5Uu-gV | simonw 9599 | 2023-02-07T23:12:40Z | 2023-02-07T23:16:20Z | OWNER | Most complicated example of a paginated query: https://latest.datasette.io/fixtures?sql=select%0D%0A++pk1%2C%0D%0A++pk2%2C%0D%0A++content%2C%0D%0A++sortable%2C%0D%0A++sortable_with_nulls%2C%0D%0A++sortable_with_nulls_2%2C%0D%0A++text%0D%0Afrom%0D%0A++sortable%0D%0Awhere%0D%0A++(%0D%0A++++sortable_with_nulls+is+null%0D%0A++++and+(%0D%0A++++++(pk1+%3E+%3Ap0)%0D%0A++++++or+(%0D%0A++++++++pk1+%3D+%3Ap0%0D%0A++++++++and+pk2+%3E+%3Ap1%0D%0A++++++)%0D%0A++++)%0D%0A++)%0D%0Aorder+by%0D%0A++sortable_with_nulls+desc%2C%0D%0A++pk1%2C%0D%0A++pk2%0D%0Alimit%0D%0A++101&p0=h&p1=r ```sql select pk1, pk2, content, sortable, sortable_with_nulls, sortable_with_nulls_2, text from sortable where ( sortable_with_nulls is null and ( (pk1 > :p0) or ( pk1 = :p0 and pk2 > :p1 ) ) ) order by sortable_with_nulls desc, pk1, pk2 limit 101 ``` Generated by this page: https://latest.datasette.io/fixtures/sortable?_next=%24null%2Ch%2Cr&_sort_desc=sortable_with_nulls The `_next=` parameter there decodes as `$null,h,r` - and those components are tilde-encoded, so this can be distinguished from an actual `$null` value which would be represented as `~24null`. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 | |
| 1421784930 | https://github.com/simonw/datasette/issues/2019#issuecomment-1421784930 | https://api.github.com/repos/simonw/datasette/issues/2019 | IC_kwDOBm6k_c5Uvrdi | simonw 9599 | 2023-02-08T01:28:25Z | 2023-02-08T01:40:46Z | OWNER | Rather than duplicate this rather awful hack: https://github.com/simonw/datasette/blob/0b4a28691468b5c758df74fa1d72a823813c96bf/datasette/views/table.py#L694-L714 I'm tempted to say that the code that calls the new pagination helper needs to ensure that the `sort` or `sort_desc` columns are selected. If it wants to ditch them later (e.g. because they were not included in `?_col=`) it can do that later once the results have come back. | {"total_count": 0, "+1": 0, "-1": 0, "laugh": 0, "hooray": 0, "confused": 0, "heart": 0, "rocket": 0, "eyes": 0} | Refactor out the keyset pagination code 1573424830 |
Advanced export
JSON shape: default, array, newline-delimited, object
CREATE TABLE [issue_comments] (
[html_url] TEXT,
[issue_url] TEXT,
[id] INTEGER PRIMARY KEY,
[node_id] TEXT,
[user] INTEGER REFERENCES [users]([id]),
[created_at] TEXT,
[updated_at] TEXT,
[author_association] TEXT,
[body] TEXT,
[reactions] TEXT,
[issue] INTEGER REFERENCES [issues]([id])
, [performed_via_github_app] TEXT);
CREATE INDEX [idx_issue_comments_issue]
ON [issue_comments] ([issue]);
CREATE INDEX [idx_issue_comments_user]
ON [issue_comments] ([user]);