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

Add missing features #24

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

timohund
Copy link

@timohund timohund commented Aug 7, 2023

This pr add's the following features:

  • Basic support for joins and whereRaw
  • Support for having and havingRaw
  • Fix recursive fetching with NextToken and allow to pass a NextToken from previous call

@timohund
Copy link
Author

timohund commented Sep 1, 2023

@norbybaru do you see the chance that this can get merged? Regarding the integration tests, please see my comment in #20 (comment) is guess the probleme here are the credentials in the pull request context.

Is there anything that can be done to get that merged?

@norbybaru norbybaru closed this Sep 14, 2023
@norbybaru norbybaru reopened this Sep 14, 2023
@@ -6,6 +6,10 @@

final class TimestreamReaderDto extends AbstractTimestreamDto
{
private ?int $maxRows = null;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the maxRows necessary? I still don't see how much useful this one is! can give an example for it?

My understanding is we can just rely on the nextToken. If the query should paginate then they can make use of Limit in the query to limit max results per page I think

@@ -6,6 +6,10 @@

final class TimestreamReaderDto extends AbstractTimestreamDto
{
private ?int $maxRows = null;

private string $nextTokenToContinueReading = '';
Copy link
Owner

@norbybaru norbybaru Sep 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't seem to find where this is being used as well!

In the loop you are always calling runQuery($params) which will have the nextToken already available in $params

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.

2 participants