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

Query pagination: total field is always greater than results count by 1 #2202

Closed
0x009922 opened this issue May 12, 2022 · 4 comments
Closed
Assignees
Labels
Bug Something isn't working good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST

Comments

@0x009922
Copy link
Contributor

Repro

  • Up Iroha: docker compose up
  • Query to it with e.g. FindAllDomains & some great pagination limit such as 15

Expected output

Expected that total will be less than limit and equals to results count.

Actual output

In YAML format:

pagination:
  page: 1
  page_size: 15
  total: 3
data:
  - id: wonderland
    accounts:
      - id: alice@wonderland
        assets:
          - account_id: alice@wonderland
            definition_id: "rose#wonderland"
            value:
              t: Quantity
              c: 13
        metadata: {}
        roles: []
    logo: ~
    metadata: {}
    asset_definitions:
      - id: "rose#wonderland"
        value_type: Quantity
        mintable: Infinitely
    triggers: 0
  - id: genesis
    accounts:
      - id: genesis@genesis
        assets: []
        metadata: {}
        roles: []
    logo: ~
    metadata: {}
    asset_definitions: []
    triggers: 0
@0x009922 0x009922 added Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST labels May 12, 2022
@appetrosyan appetrosyan added the good first issue Good for newcomers label May 19, 2022
@Erigara Erigara self-assigned this Jun 7, 2022
Erigara added a commit to Erigara/iroha that referenced this issue Jun 8, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
@Erigara
Copy link
Contributor

Erigara commented Jun 8, 2022

So right now we are calculating total field as number of underneath expressions before we applying pagination.
This means that the length of the vector will be equal to sum of the lengths of the elements plus one.
The question is, what behavior do we want from the total?

@appetrosyan
Copy link
Contributor

[8 Jun 2022] Bogdan Mingela (@Mingela)

Pagination is used for traversing the elements and reflecting them correctly, specifically in a web app. Imagine filtering any entity by part of the name provided. The result must be a page of the list of elements and we should know how many got filtered in total to have a convenient navigation. I don't get how topmost calculation solves that. Seems we should calculate all the internals.

Since the behaviour is the one that SDK developers want, there's no point in subtracting 1 on the server side.

@arndey @0x009922 @posplaw, please be advised.

@appetrosyan
Copy link
Contributor

appetrosyan commented Jun 8, 2022

As per yet another miscommunication, I believe that we need to change the implementation such that the total is the number of values returned in the top-most level of an iterable container before pagination.

I.e.
if query returned
[[1], [2,3], [4,5,6]]
which are paginated one by one, total is 3.

@Mingela Please confirm that this is what you want.

@Mingela
Copy link
Contributor

Mingela commented Jun 8, 2022

As per yet another miscommunication, I believe that we need to change the implementation such that the total is the number of values returned in the top-most level of an iterable container before pagination.

I.e. if query returned [[1], [2,3], [4,5,6]] which are paginated one by one, total is 3.

@Mingela Please confirm that this is what you want.

I think so, though the example is a bit confusing, I don't expect a list of lists returned as a query result.
A page containing smth like [{1},{2:[3,4]},{5,6:[7,8]}] would be more precise to me. Total is 3 in this case as well.

Erigara added a commit to Erigara/iroha that referenced this issue Jun 8, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Erigara added a commit to Erigara/iroha that referenced this issue Jun 8, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
appetrosyan pushed a commit to Erigara/iroha that referenced this issue Jun 9, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
appetrosyan pushed a commit that referenced this issue Jun 9, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
BAStos525 pushed a commit to BAStos525/soramitsu-iroha that referenced this issue Jul 8, 2022
Signed-off-by: Shanin Roman <shanin1000@yandex.ru>
Signed-off-by: BAStos525 <jungle.vas@yandex.ru>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working good first issue Good for newcomers iroha2-dev The re-implementation of a BFT hyperledger in RUST
Projects
None yet
Development

No branches or pull requests

4 participants