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.

Sign up for GitHub

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

Jump to bottom

Query Backfill Features #8791

Merged
merged 6 commits into from Jun 22, 2022
Merged

Query Backfill Features #8791

merged 6 commits into from Jun 22, 2022

Conversation

anshuldavid13
Copy link
Contributor

No description provided.

@anshuldavid13 anshuldavid13 requested a review from a team as a code owner June 16, 2022 17:30
@anshuldavid13 anshuldavid13 self-assigned this Jun 16, 2022
jskeet
jskeet reviewed Jun 20, 2022
View reviewed changes
Copy link
Collaborator

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Okay, now that we know the behavior, please could you modify the documentation comments for the parameters to Filter.In and Filter.NotIn so that users can know what to expect if the value is null?

apis/Google.Cloud.Datastore.V1/Google.Cloud.Datastore.V1.IntegrationTests/DatastoreDbTest.cs Outdated Show resolved Hide resolved
@jskeet
Copy link
Collaborator

jskeet commented Jun 20, 2022

I've tried running the tests, and they fail with errors of:

  • IN requires an ArrayValue.
  • NOT_IN requires an ArrayValue.

So I think we should instead change the implementation to prevent callers from providing a null argument to start with.

jskeet
jskeet approved these changes Jun 22, 2022
View reviewed changes
Copy link
Collaborator

@jskeet jskeet left a comment

Choose a reason for hiding this comment

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

Right, this looks appropriate now. I suggest you put it into a single commit with a suitable comment, edit the PR title for posterity, then we can merge.

@anshuldavid13 anshuldavid13 merged commit daf7776 into googleapis:main Jun 22, 2022
@anshuldavid13 anshuldavid13 changed the title WIP Query Backfill Features Sep 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jskeet jskeet jskeet approved these changes

Assignees

@anshuldavid13 anshuldavid13

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@anshuldavid13 @jskeet