piccolo-orm / piccolo_admin

A powerful web admin for your database.
https://piccolo-orm.com/ecosystem/
MIT License
316 stars 37 forks source link

How well do nested arrays work? #369

Open dantownsend opened 7 months ago

dantownsend commented 7 months ago

When using a nested array with Piccolo Admin, for example:

from piccolo.table import Table
from piccolo.columns import Array, BigInt

class MyTable(Table):
    my_column = Array(Array(BigInt()))

This error is raised by FastAPI:

AssertionError: Param: my_column can only be a request body, using Body()

I'm not sure how hard it will be to solve, but it would be nice to have basic nested array support.

Related to https://github.com/piccolo-orm/piccolo/issues/936

sinisaos commented 7 months ago

@dantownsend With your latest changes to create_pydantic_model the schema is correct for a nested list

schema

We are using Query for filter in FastAPIWrapper for Piccolo Admin, but I don't think FastAPI supports nested list in query parameters. This error says that we should use the FastAPI Body, but the GET method basically does not use the body of the request and is not recommended. Having nested arrays in Piccolo Admin will be great, but I don't know how to achieve this due to FastAPI limitations. Sorry if I'm wrong.

dantownsend commented 7 months ago

@sinisaos Yeah, I think you're right.

We could modify Piccolo Admin to send filters for multidimensional arrays via the body, but I don't think the solution is ideal.

I think I would rather disable filtering for multidimensional arrays, or instead accept a string, which has it's own mini array filtering syntax.

sinisaos commented 6 months ago

@dantownsend I played around a bit with nested arrays and here is the working branch. You probably have a better solution. I don't know if this is the best way, but I didn't know a better and easier way to dynamically nest arrays to create, update and filter rows. Along with the changes to Piccolo Admin, changes should be made to the Piccolo API like this

# in piccolo_api/crud/endpoints.py
def _parse_params(self, params: QueryParams) -> t.Dict[str, t.Any]:
    ...
    for key, value in params_map.items():
        # remove all [] or [0][1] or [0][1][2] etc. from
        # endings of keys in array columns.
        # Used to filter nested lists.
        key = key.split("[")[0]
        if key in array_columns:
            key = key
        elif len(value) == 1:
            value = value[0]

        output[key] = value

Sorry if that's not good, it's just an idea how to solve this edge case problem with nested arrays.

dantownsend commented 6 months ago

With the changes to Piccolo API, filtering should hopefully work for simple cases: https://github.com/piccolo-orm/piccolo_api/pull/270

There are challenges with multidimensional arrays still - the UI doesn't work particularly well for adding / creating. I was thinking of just replacing the usual array input with a textarea if it's multidimensional.

sinisaos commented 6 months ago

With the changes to Piccolo API, filtering should hopefully work for simple cases: piccolo-orm/piccolo_api#270

I forgot to write it down but all the changes I made were made on top of that PR. The changes for the Piccolo API that I wrote in the previous comment is just an addition to your already made changes.

There are challenges with multidimensional arrays still - the UI doesn't work particularly well for adding / creating. I was thinking of just replacing the usual array input with a textarea if it's multidimensional.

Textarea is a good idea and I'm interested to see how it will look, but in a textarea you will also have problems with dynamically nesting arrays with number of dimensions. I just tried to mimic what is happening in the Fastapi OpenAPI docs (I posted here just as idea)