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

?_extra=columns parameter causes all rows to return "column-name": "column-name" #2230

Closed
Tracked by #2251
simonw opened this issue Jan 8, 2024 · 6 comments
Closed
Tracked by #2251
Labels
Milestone

Comments

@simonw
Copy link
Owner

simonw commented Jan 8, 2024

Found a Datasette 1.0 bug: the ?_extra=columns parameter causes all rows to return `"column-name": "column-name": https://latest.datasette.io/fixtures/attraction_characteristic.json?_extra=columns&_size=1

{
  "ok": true,
  "next": "2,2",
  "columns": [
    "pk",
    "name"
  ],
  "rows": [
    {
      "pk": "pk",
      "name": "name"
    }
  ],
  "truncated": false
}

Should return this:

{
  "ok": true,
  "next": "2,2",
  "columns": [
    "pk",
    "name"
  ],
  "rows": [
    {
      "pk": 2,
      "name": "Paranormal"
    }
  ],
  "truncated": false
}

Originally posted by @simonw in simonw/datasette-search-all#19 (comment)

@simonw simonw changed the title ?_extra=columns parameter causes all rows to return "column-name": "column-name" ?_extra=columns parameter causes all rows to return "column-name": "column-name" Jan 8, 2024
@simonw simonw added the bug label Jan 8, 2024
@simonw simonw added this to the Datasette 1.0 milestone Jan 8, 2024
@simonw
Copy link
Owner Author

simonw commented Jan 8, 2024

The bug isn't in the extra itself:

async def extra_columns():
"Column names returned by this query"
return columns

It must be in the way the other code interacts with it.

@simonw
Copy link
Owner Author

simonw commented Jan 8, 2024

I have a hunch the bug may be in here somewhere:

# Deal with the _shape option
shape = args.get("_shape", "objects")
# if there's an error, ignore the shape entirely
data["ok"] = True
if error:
shape = "objects"
status_code = 400
data["error"] = error
data["ok"] = False
if truncated is not None:
data["truncated"] = truncated
if shape == "arrayfirst":
if not data["rows"]:
data = []
elif isinstance(data["rows"][0], sqlite3.Row):
data = [row[0] for row in data["rows"]]
else:
assert isinstance(data["rows"][0], dict)
data = [next(iter(row.values())) for row in data["rows"]]
elif shape in ("objects", "object", "array"):
columns = data.get("columns")
rows = data.get("rows")
if rows and columns:
data["rows"] = [dict(zip(columns, row)) for row in rows]
if shape == "object":
shape_error = None
if "primary_keys" not in data:
shape_error = "_shape=object is only available on tables"
else:
pks = data["primary_keys"]
if not pks:
shape_error = (
"_shape=object not available for tables with no primary keys"
)
else:
object_rows = {}
for row in data["rows"]:
pk_string = path_from_row_pks(row, pks, not pks)
object_rows[pk_string] = row
data = object_rows
if shape_error:
data = {"ok": False, "error": shape_error}
elif shape == "array":
data = data["rows"]

@simonw
Copy link
Owner Author

simonw commented Jan 8, 2024

In the debugger:

-> columns = data.get("columns")
(Pdb) list
 65  	        else:
 66  	            assert isinstance(data["rows"][0], dict)
 67  	            data = [next(iter(row.values())) for row in data["rows"]]
 68  	    elif shape in ("objects", "object", "array"):
 69  	        breakpoint()
 70  ->	        columns = data.get("columns")
 71  	        rows = data.get("rows")
 72  	        if rows and columns:
 73  	            data["rows"] = [dict(zip(columns, row)) for row in rows]
 74  	        if shape == "object":
 75  	            shape_error = None
(Pdb) data
{'ok': True, 'next': None, 'columns': ['id', 'content', 'content2'], 'rows': [{'id': '1', 'content': 'hey', 'content2': 'world'}], 'truncated': False}

@simonw
Copy link
Owner Author

simonw commented Jan 8, 2024

The bug is here:

rows = data.get("rows")
if rows and columns:
data["rows"] = [dict(zip(columns, row)) for row in rows]
if shape == "object":

This code assumes that row is a list of values - but in this case it's not, it is a list of dicts already.

@simonw
Copy link
Owner Author

simonw commented Jan 8, 2024

Surprising bug: I'm seeing the ID returned as a string when I expect an integer:

E           'rows': [                        'rows': [                     
E             {                                {                           
E               'content': 'hey',                'content': 'hey',         
E               'content2': 'world',             'content2': 'world',      
E               'id': '1',                       'id': 1,                  
E             },                               },                          
E           ],                               ],                            

@simonw
Copy link
Owner Author

simonw commented Jan 8, 2024

No, it's OK - that table in fixtures does have strings for primary keys: https://latest.datasette.io/fixtures/primary_key_multiple_columns

CREATE TABLE primary_key_multiple_columns (
  id varchar(30) primary key,
  content text,
  content2 text
);

@simonw simonw closed this as completed in 0b2c6a7 Jan 8, 2024
@simonw simonw mentioned this issue Feb 5, 2024
13 tasks
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

1 participant