-
Notifications
You must be signed in to change notification settings - Fork 103
Description
Environment information
@aws-amplify/auth-construct: 1.8.1
@aws-amplify/backend: 1.16.1
@aws-amplify/backend-ai: Not Found
@aws-amplify/backend-auth: 1.7.1
@aws-amplify/backend-cli: 1.8.0
@aws-amplify/backend-data: 1.6.1
@aws-amplify/backend-deployer: 2.1.3
@aws-amplify/backend-function: 1.14.1
@aws-amplify/backend-output-storage: 1.3.1
@aws-amplify/backend-secret: 1.4.0
@aws-amplify/backend-storage: 1.4.1
@aws-amplify/cli-core: 2.2.1
@aws-amplify/client-config: 1.8.0
@aws-amplify/data-construct: 1.16.3
@aws-amplify/data-schema: 1.21.1
@aws-amplify/deployed-backend-client: 1.8.0
@aws-amplify/form-generator: 1.2.4
@aws-amplify/model-generator: 1.2.0
@aws-amplify/platform-core: 1.10.0
@aws-amplify/plugin-types: 1.11.0
@aws-amplify/sandbox: 2.1.2
@aws-amplify/schema-generator: 1.4.0
@aws-cdk/toolkit-lib: 1.1.1
aws-amplify: 6.15.5
aws-cdk-lib: 2.214.0
typescript: 5.9.2
No AWS environment variables
No CDK environment variables
Describe the bug
Hi Guys,
We observed this, then suspect the issues after double check Postgress SQL documents that this can happen, which states yes.
Which means that the programming wrapping/generating the SQL query has to ensure deterministic behavior is enforced the ordering otherwise the database ordering can change.
As you see Amplify is Generating SQL statements.
Now if you go read SQL Postgres driver documentation, the record ordering is not deterministic and intermixed with the current transactions and things in progress based on Deletes and Updates that have recently gone off, can affect ordering.
The database can based the orders of records return by a select statement before and after could vary in the logical record position offset from 0.
This is basically what we have been seeing for our unit test cases against the database, at random times the ordering of records changes in the results set.
It’s the same data in the results set, but the order of the records has changed.
The issues with this is that for unit test break and then try again get stuck return that ordering, then at point in time just swap back to the old ordering.
Additionally, this could also be a result of the SQL database query planning come to think of it chosen different plans to retrieve the information, to use or not to use index or different index, by not having explicit order by clauses, typically based on something similar to the model.list({filter: fields....}) in narrow field choice of index, it not going to be as fast or deterministic.
Query planner runs on statical methods to choose how to build the final dataset, and that doesn’t care about ordering
Can change from query plan to query plan. There no guaranteed ordering from different query plans.
They can change at any time, which plan is being used and changes the final output of the ordering of records breaking pagination, if no order by clause.
Use you can also ways fall back to sorting by primary key, but that is not ideal, as would be massive post ordering if compound index was used. So everything should have sort/order by clause, which should implicitly be filter fields. Because everything needs order by clause for pagination to be predictable.. can't go writing loads special functions all the time for everything, what crazy overhead..
Additional, there algorithms for sorting and that, where some are deterministic and others are not deterministic.
The database could based on the outcome of the desired results make use of non-deterministic algorithms, when
doesn’t need to rely on specific ordering of input for the next input transform stage to speed things up.
So without order by clause, one has left the door wide open to a lot things possible going wrong with database integration.
and the query being optimized, by query planner all the time.
This is fine given, if writing backend API, were know records will never be more than page size to process, but if the table had 500 records, this meaning 5 pages, then there potentially a problem here.
Because post select you break things up into pages and the ordering of records on the boundaries of pages could change, if the ordering is not deterministic.
A record that was in Page1, could now be in Page2 and visa versa on successive API calls, because database re-organized optimized things again between queries.
So basically without amplify explicitly specify order by clause by the primary key implicitly for each table, we have paging problem. We have all our fields mark as id() and also specified .identifier(['id']).
If you doing a webpages you have to write some form of order by clause,
After which you would then limit and offset the results for predictable deterministic behavior, which perfect what we want for pagination.
So since we using and API to get records from the database via an API, which just like webpage report would do, you would have to add some form of ordering, to ensure the pagination is consistent.
If you don’t the following side effects would existing in your current pagination implementation.
- Duplicate Records could be return, was in Page 1 now its in Page 2, due to the database re-ordering.
- Missing Records, because what was in Page 2, after Page 1 is read is now in Page1, so when never read the records that between API calls moved
from page 2 to page 1.
[Amplify Gen2 Schema Model Complexity Leads to TS2590 Error in TypeScript Client - Handling Large Models and Improvements for TS](Amplify Gen2 Schema Model Complexity Leads to TS2590 Error in TypeScript Client amplify-data#424 (comment))
So you can imagine, that basically every backend call one makes we going to have to add order by clause, but that basically means custom query in amplify for every single thing. Because there is not ability to just add sort to any list command.
The issues now with this is probably run into the CloudFormation appsync 2500 resource limit very quickly, as we have seen how all of this is being wired, hit limits already, this would be another new AppSync endpoint and another CloudFormation resource for each order by custom query.
[How to split Amplify project into multiple stacks. Error - Limit on the number of resources in a single stack operation exceeded](aws-amplify/amplify-cli#13536)
Could you please advise and provide some reasonable mitigation measure and what next steps might need to look like from AWS and what clients could do in the mean time as mitigation or would Amplify release small hotpatch allowing list({orderby:}) field and also implicit look at adding order by the primary key,id or identifier, field of a model to get deterministic behavior of record ordering be return from the database, such pagination would be reliable and deterministic then.
Reproduction steps
I guess you would write simulation against Postgress database, with rapidly, writing, update, delete records.
Then running API against the database, ensure that all the records are present for the full table spread across all the pages. I would imaging say target 200, 300 or 2000 records and then validate pagination algorithm, but check expected dataset against, after updates, deletes are made to via API calls. That the record re-ordering doesn't results record changing pages on the boundaries, less pages more chance maybe of swapping pages, but who knows, you need insights into the database algorithms and code implementation for better hunch than that, what cause to trigger more often than not to expose the no deterministic behavior. Ensure that you have other tracking additional fields added, so you can correlate the order of the records changing.