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

SearchCriteriaBetween fails when using a DateTime value #524

Closed
wayneswyfft opened this issue Jun 18, 2024 · 11 comments
Closed

SearchCriteriaBetween fails when using a DateTime value #524

wayneswyfft opened this issue Jun 18, 2024 · 11 comments
Assignees
Labels
Bug This change resolves a defect
Milestone

Comments

@wayneswyfft
Copy link

Using SearchCriteriaBetween you end up with a query like last_event_time BETWEEN "2024-06-17T00:00:00.000Z" AND "2024-06-18T00:00:00.000Z"

Which results in the error message - operator >= is incompatible with DATETIME, STRING

The query needs to look like last_event_time BETWEEN TIMESTAMP "2024-06-17T00:00:00.000Z" AND TIMESTAMP "2024-06-18T00:00:00.000Z"

@Jericho Jericho self-assigned this Jun 19, 2024
@Jericho Jericho added the Bug This change resolves a defect label Jun 19, 2024
@Jericho Jericho added this to the 0.109.0 milestone Jun 19, 2024
@Jericho
Copy link
Owner

Jericho commented Jun 19, 2024

I confirm that I can reproduce the problem. In fact, I just realized that the problem occurs whenever you use a DateTime value regardless of the search operator (in other words, it's not specific to the BETWEEN search operator).

Here's the snippet I used to reproduce:

var modifiedDuringPriorYearCriteria = new SearchCriteriaGreaterThan(ContactsFilterField.ModifiedOn, DateTime.UtcNow.AddYears(-1));
var contactsModifiedDuringPriorYearResult = await client.Contacts.SearchAsync(modifiedDuringPriorYearCriteria, cancellationToken).ConfigureAwait(false);
await log.WriteLineAsync($"Found {contactsModifiedDuringPriorYearResult.Length} contacts modified during the last year").ConfigureAwait(false);

I should have a fix shortly.

@Jericho
Copy link
Owner

Jericho commented Jun 19, 2024

Quick update:
I figured out why we have this issue in the StrongGrid library. SendGrid has two distinct query languages. There's the "old" one (AKA "v1") which is still used when searching for contacts or email activities and there is the "new" one (AKA "v2") which is used when segmenting a list. I designed StrongGrid to support both versions but I did not notice that DateTime values are handled differently in the two versions. Version 1 (which is the version used when searching for contacts and email activities) requires the 'TIMESTAMP' keyword in front of the data value expressed as a string (like this example: updated_at < TIMESTAMP "2024-06-19T00:00:00Z"). However, you must not include this keyword in a v2 query like this example: SELECT c.contact_id, c.updated_at FROM contact_data AS c WHERE c.updated_at > '2023-06-19T13:57:50Z'.

If I simply modify our library to add the "TIMESTAMP" keyword, it will fix v1 queries but it will break v2 queries. Which means that I need to figure out a way handle DateTime differently depending on the query language version.

All this to say that the fix will not be as simple as I originally thought. That's the bad news.

The good news is that there's a temporary workaround for you while you wait for me to fix the problem: you can manually craft you own query expressed as a string when invoking the Contacts.Search(...) method like this example:

var myQuery = "last_event_time BETWEEN TIMESTAMP \"2024-06-17T00:00:00.000Z\" AND TIMESTAMP \"2024-06-18T00:00:00.000Z\"";
var searchResult = await client.Contacts.SearchAsync(myQuery, cancellationToken).ConfigureAwait(false);
await log.WriteLineAsync($"Found {searchResult.Length} contacts").ConfigureAwait(false);

Let me know if this helps.

@wayneswyfft
Copy link
Author

Thanks for the update, I was unaware of the ability to pass a string to SearchAsync. FYI I am calling client.EmailActivities.SearchAsync()
I was able to get past this issue in my context by creating a SearchCriteriaDateBetween class inheriting from SearchCriteriaBetween and overriding ConvertValueToString()

@Jericho
Copy link
Owner

Jericho commented Jun 19, 2024

I was unaware of the ability to pass a string to SearchAsync.

This is how developers can specify advanced queries with features that StrongGrid doesn't support such as, for example, using some built-in functions like 'lower()'.

FYI I am calling client.EmailActivities.SearchAsync()

Both searching for contacts and email activities use "old" v1 queries. That's why I was able to reproduce the problem by invoking Contacts.SearchAsync. The new v2 queries are used when you create a segment.

I was able to get past this issue in my context by creating a SearchCriteriaDateBetween class inheriting from SearchCriteriaBetween and overriding ConvertValueToString()

Very clever! Hopefully, this will be made obsolete by the improvement I'm working on.

@Jericho
Copy link
Owner

Jericho commented Jun 19, 2024

I published a beta package to my personal NuGet feed called 0.109.0-handle-dates-in-0021.nupkg. Let me know if you have an opportunity to test it. Dates should now be properly handled no matter the query version. Don't forget to remove your custom SearchCriteriaDateBetween class though to ensure your custom logic does not take precedence over the fix I am publishing.

@Jericho
Copy link
Owner

Jericho commented Jun 19, 2024

It just occurred to me that the workaround I suggested earlier won't work in your situation. I added public Task<Contact[]> SearchAsync(string query, CancellationToken cancellationToken = default) to the Contacts resource, allowing developers to craft their own queries when searching for contacts, but I did not do the same for the EmailActivities resource.

The fix I'm working on (which you're able to beta test) renders this a moot point but I should raise a new issue to add this enhancement to the EmailActivities resource.

@wayneswyfft
Copy link
Author

I've tested 0.109.0 and it works for my scenario. Thanks for the quick update.

@Jericho
Copy link
Owner

Jericho commented Jun 20, 2024

🎉 This issue has been resolved in version 0.109.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

@Jericho
Copy link
Owner

Jericho commented Jun 21, 2024

Excellent. Thank you for confirming. I've published v0.109.0 to NuGet. It includes not only the fix for this issue but also the improvement discussed in #527 which allows you to provide your own queries as a string in case you ever need to utilize a feature of SendGrid's query engine that StrongGrid does not support.

@wayneswyfft
Copy link
Author

wayneswyfft commented Jun 21, 2024 via email

@Jericho
Copy link
Owner

Jericho commented Jun 22, 2024

I sent him a quick note to thank him for the sponsorship. Thank you for advocating for me!

Don't hesitate to reach out if you need anything (new feature, fix, general question, etc.).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug This change resolves a defect
Projects
None yet
Development

No branches or pull requests

2 participants