Skip to content

Add permalink virtual field to items table#6

Open
xavdid wants to merge 1 commit intodogsheep:mainfrom
xavdid:main
Open

Add permalink virtual field to items table#6
xavdid wants to merge 1 commit intodogsheep:mainfrom
xavdid:main

Conversation

@xavdid
Copy link

@xavdid xavdid commented Mar 26, 2023

I added a virtual column (no storage overhead) to the output that easily links back to the source. It works nicely out of the box with datasette:

I got bit a bit by simonw/sqlite-utils#411, so I went with a manual table_xinfo and creating the table via execute. Happy to adjust if that issue moves, but this seems like it works.

I also added my best-guess instructions for local development on this package. I'm shooting in the dark, so feel free to replace with how you work on it locally.

.pytest_cache
*.egg-info No newline at end of file
*.egg-info
build
Copy link
Author

Choose a reason for hiding this comment

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

this was created as a result of pip install . - if that's not the correct way to work locally, then we can remove.

As you make changes to the code, you can re-run it using:

```sh
.venv/bin/hacker-news-to-sqlite
Copy link
Author

Choose a reason for hiding this comment

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

This whole section is basically a total guess. If you have a different process (that you can either document or link) that would be super helpful!

)
# includes hidden columns
all_column_names = {
c[1] for c in db.execute("PRAGMA table_xinfo([items])").fetchall()
Copy link
Author

Choose a reason for hiding this comment

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

Generated columns are hidden and are thus not included in db['items'].column_dict

c[1] for c in db.execute("PRAGMA table_xinfo([items])").fetchall()
}
if "permalink" not in all_column_names:
db.execute(
Copy link
Author

Choose a reason for hiding this comment

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

Could be improved by a resolution on simonw/sqlite-utils#411

Copy link
Author

Choose a reason for hiding this comment

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

Also I was mulling this over a bit - making the table virtual provides backwards compatibility, but it might be better to create a real column for new databases. It'll take up space storing a lot of nearly-identical strings, but it won't incur repeated computation at runtime.

I was thinking about it in the context of adding a num_children column to make interacting with the string kids column easier. This can be done in pure sqlite as another virtual column, but it would be easier to pre-compute it in new databases (and provide a virtual table for existing ones)

@xavdid
Copy link
Author

xavdid commented Mar 29, 2023

@simonw can you take a look when you have a chance?

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.

1 participant