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

[Pg] AWS Data API fixes #1939

Closed
wants to merge 14 commits into from
Closed

[Pg] AWS Data API fixes #1939

wants to merge 14 commits into from

Conversation

hugo082
Copy link
Contributor

@hugo082 hugo082 commented Feb 28, 2024

This PR address multiple issues on AWS Data API driver, each via a separate commit.

1. Reconciliation of AwsDataApiSession with PgSession base class

The third parameter of AwsDataApiSession.prepareQuery should be the name to stay compatible with the postgres query builder especially the .prepare(name: string) method. - #1171

  • The transactionId is now the fifth one. Anyway it is used internally only. Note that it is also possible to remove the parameter as we have the value on the session instance properties.
  • the name parameter is ignored (as on neon-http driver & some others)

2. Fix the transaction id propagation

If the transaction id is not provided on the query, the session should use the current one, the one stored on the instance.

3. Fix nested transactions

  • await for the savepoint to be created / rollback / released before continuing
  • await for the transaction handler to be executed for releasing the savepoint
  • do not use parameters on savepoints queries as it is not supported by AWS Data API - [BUG]: AWS Data API nested transactions - savepoints #1936

4. Tests fixes

Fix outdated tests (mainly because the returned values doesn't match the type on raw execution).

We probably should update the AwsDataApiPgQueryResultHKT.type: aws.ExecuteStatementCommandOutput but i'm not sure what would be the right value. Probably something similar to the neon-serverless version NeonQueryResultHKT.type: QueryResult<Assume<this['row'], QueryResultRow>>;

@hugo082
Copy link
Contributor Author

hugo082 commented Feb 28, 2024

AWS Data API tests suite

Before After
Screenshot 2024-02-28 at 16 55 09 Screenshot 2024-02-28 at 18 11 33

@hugo082 hugo082 changed the title AWS Data API fixes [Pg] AWS Data API fixes Feb 28, 2024
@jakeleventhal
Copy link

You should check out these PRs as well:
#1933
#1931
#1911

@hugo082
Copy link
Contributor Author

hugo082 commented Mar 6, 2024

Hi @dankochetov @AndriiSherman 👋

What are the next steps to merge these fixes? Do you think there is any change to merge them during the month?

@dankochetov dankochetov self-assigned this Mar 20, 2024
@dankochetov
Copy link
Contributor

@hugo082 please sign your commits
https://docs.github.com/articles/about-gpg/

@hugo082
Copy link
Contributor Author

hugo082 commented Mar 21, 2024

@hugo082 please sign your commits docs.github.com/articles/about-gpg

@dankochetov done ✅

@hugo082
Copy link
Contributor Author

hugo082 commented Apr 3, 2024

@dankochetov, any updates?

@dankochetov
Copy link
Contributor

@hugo082 yes, I'm testing it myself and making adjustments, hoping to merge in the next few days

@dankochetov
Copy link
Contributor

I've pushed this PR with my changes into a separate branch, will continue it in #2119

@dankochetov dankochetov closed this Apr 6, 2024
@dankochetov dankochetov mentioned this pull request Apr 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants