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

Datasette row URLs break for binary data as a primary key #2419

Open
simonw opened this issue Sep 3, 2024 · 3 comments
Open

Datasette row URLs break for binary data as a primary key #2419

simonw opened this issue Sep 3, 2024 · 3 comments
Labels

Comments

@simonw
Copy link
Owner

simonw commented Sep 3, 2024

Spotted this while browsing a FTS table:

CleanShot 2024-09-03 at 14 33 52@2x

Schema is:

CREATE TABLE 'releases_fts_idx'(
  segid,
  term,
  pgno,
  PRIMARY KEY(segid, term)
) WITHOUT ROWID;

Got a 404 error when I clicked the link to e.g. /content/releases_fts_idx/3,b~270thei~27

@simonw simonw added the bug label Sep 3, 2024
@simonw
Copy link
Owner Author

simonw commented Sep 3, 2024

I think the bigger problem here is that a row page with a binary primary key cannot even be linked to.

Maybe need some kind of way of representing hex() in a URL to a row page?

@simonw
Copy link
Owner Author

simonw commented Sep 3, 2024

Relevant code:

class RowView(DataView):
name = "row"
async def data(self, request, default_labels=False):
resolved = await self.ds.resolve_row(request)

Which calls:

datasette/datasette/app.py

Lines 1643 to 1651 in 2170269

async def resolve_row(self, request):
db, table_name, _ = await self.resolve_table(request)
pk_values = urlsafe_components(request.url_vars["pks"])
sql, params, pks = await row_sql_params_pks(db, table_name, pk_values)
results = await db.execute(sql, params, truncate=True)
row = results.first()
if row is None:
raise RowNotFound(db.name, table_name, pk_values)
return ResolvedRow(db, table_name, sql, params, pks, pk_values, results.first())

Which calls:

async def row_sql_params_pks(db, table, pk_values):
pks = await db.primary_keys(table)
use_rowid = not pks
select = "*"
if use_rowid:
select = "rowid, *"
pks = ["rowid"]
wheres = [f'"{pk}"=:p{i}' for i, pk in enumerate(pks)]
sql = f"select {select} from {escape_sqlite(table)} where {' AND '.join(wheres)}"
params = {}
for i, pk_value in enumerate(pk_values):
params[f"p{i}"] = pk_value
return sql, params, pks

@simonw simonw changed the title Datasette breaks for binary data as a primary key Datasette URLs break for binary data as a primary key Sep 3, 2024
@simonw simonw changed the title Datasette URLs break for binary data as a primary key Datasette row URLs break for binary data as a primary key Sep 3, 2024
@asg017
Copy link
Collaborator

asg017 commented Sep 3, 2024

Sometimes people store UUIDs or ULIDs as compact BLOBs instead of their more verbose TEXT format.

(don't really see much value in that personally but I guess it's something to consider)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants