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

How well do nested arrays work? #369

Open
dantownsend opened this issue Mar 4, 2024 · 5 comments
Open

How well do nested arrays work? #369

dantownsend opened this issue Mar 4, 2024 · 5 comments
Labels
bug Something isn't working
Projects

Comments

@dantownsend
Copy link
Member

dantownsend commented Mar 4, 2024

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 piccolo-orm/piccolo#936

@dantownsend dantownsend added the bug Something isn't working label Mar 4, 2024
@dantownsend dantownsend added this to To do in Bugs via automation Mar 4, 2024
@sinisaos
Copy link
Member

sinisaos commented Mar 5, 2024

@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
Copy link
Member Author

@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
Copy link
Member

@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
Copy link
Member Author

With the changes to Piccolo API, filtering should hopefully work for simple cases: piccolo-orm/piccolo_api#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
Copy link
Member

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Bugs
To do
Development

No branches or pull requests

2 participants