-
-
Notifications
You must be signed in to change notification settings - Fork 24
Elasticsearch DateMath support #113
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
Conversation
Extends the date range parsing functionality to support wildcard characters, allowing for open-ended date ranges. Implements a new `WildcardPartParser` to handle "*" characters, representing either the minimum or maximum DateTimeOffset value based on the context. Updates the `TwoPartFormatParser` to correctly handle bracketed date ranges and wildcard characters.
Adds support for "TO" delimiter and bracket syntax to the two-part date range parser. This allows for more flexible date range input formats, including those used by Elasticsearch.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds Elasticsearch DateMath support to the DateTime extensions library, specifically implementing bracket syntax ([
and {
), TO
delimiter support, and wildcard (*
) functionality for date range queries. The implementation follows Elasticsearch's query-dsl-query-string-query range syntax patterns.
- Adds wildcard part parser for handling
*
in date ranges (maps to DateTime.MinValue/MaxValue) - Extends TwoPartFormatParser to support Elasticsearch bracket syntax and
TO
delimiter - Comprehensive test coverage for all new syntax patterns including edge cases
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
src/Exceptionless.DateTimeExtensions/FormatParsers/FormatParsers/TwoPartFormatParser.cs |
Updated regex patterns to support Elasticsearch bracket syntax ([ ] { } ) |
src/Exceptionless.DateTimeExtensions/FormatParsers/FormatParsers/PartParsers/WildcardPartParser.cs |
New parser implementation for wildcard (* ) support in date ranges |
tests/Exceptionless.DateTimeExtensions.Tests/FormatParsers/TwoPartFormatParserTests.cs |
Added test cases for new syntax patterns including TO delimiter, brackets, and wildcards |
tests/Exceptionless.DateTimeExtensions.Tests/FormatParsers/PartParsers/WildcardPartParserTests.cs |
Comprehensive test suite for wildcard parser functionality |
...ceptionless.DateTimeExtensions/FormatParsers/FormatParsers/PartParsers/WildcardPartParser.cs
Outdated
Show resolved
Hide resolved
Adds a new entry to .gitignore to prevent tracking of specific IDE configuration files related to the project. This ensures a cleaner repository by excluding auto-generated files and preventing accidental commits of personal IDE settings.
Adds `RegexOptions.Compiled` to regex definitions in the `TwoPartFormatParser` to improve performance.
Introduces tests for parsing date ranges using various delimiters including dashes, "TO", and Elasticsearch bracket syntax. Also, adds tests for wildcard support in date range parsing. Addresses and updates existing wildcard parser tests.
Implements parsing of Elasticsearch-style date math expressions within date ranges, offering flexible and precise date calculations. This includes support for: - Anchors (now, explicit dates) - Operations (+, -, /) - Units (y, M, w, d, h, m, s) - Timezone handling Updates the DateTimeRange parsing to support bracket notation and wildcards.
...ceptionless.DateTimeExtensions/FormatParsers/FormatParsers/PartParsers/DateMathPartParser.cs
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, but really wish you had kept the DateMath class separate so it could be used by itself.
...ceptionless.DateTimeExtensions/FormatParsers/FormatParsers/PartParsers/DateMathPartParser.cs
Outdated
Show resolved
Hide resolved
Refactors the DateMathPartParser to enhance performance and accuracy in parsing Elasticsearch date math expressions. - Optimizes explicit date parsing by using length-based format selection and pre-compiled regex. - Improves timezone handling and format ordering for better parsing. - Adds comprehensive tests to ensure correctness.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated 9 comments.
public class TwoPartFormatParser : IFormatParser | ||
{ | ||
private static readonly Regex _beginRegex = new(@"^\s*", RegexOptions.Compiled); | ||
private static readonly Regex _beginRegex = new(@"^\s*(?:[\[\{])?\s*", RegexOptions.Compiled); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated regexes consume opening and closing brackets/braces but do not capture which type was used; without capturing, the parser cannot distinguish inclusive [] from exclusive {} semantics (Elasticsearch treats {} as exclusive). This results in silently treating exclusive bounds as inclusive. Either (a) remove brace support until exclusivity is implemented, or (b) capture bracket type and adjust start/end boundaries accordingly (e.g., shift by smallest representable unit). Example: modify begin/end patterns to capture the delimiter type and pass it through parsing logic.
Copilot uses AI. Check for mistakes.
private static readonly Regex _beginRegex = new(@"^\s*(?:[\[\{])?\s*", RegexOptions.Compiled); | ||
private static readonly Regex _delimiterRegex = new(@"\G(?:\s*-\s*|\s+TO\s+)", RegexOptions.Compiled | RegexOptions.IgnoreCase); | ||
private static readonly Regex _endRegex = new(@"\G\s*$", RegexOptions.Compiled); | ||
private static readonly Regex _endRegex = new(@"\G\s*(?:[\]\}])?\s*$", RegexOptions.Compiled); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The updated regexes consume opening and closing brackets/braces but do not capture which type was used; without capturing, the parser cannot distinguish inclusive [] from exclusive {} semantics (Elasticsearch treats {} as exclusive). This results in silently treating exclusive bounds as inclusive. Either (a) remove brace support until exclusivity is implemented, or (b) capture bracket type and adjust start/end boundaries accordingly (e.g., shift by smallest representable unit). Example: modify begin/end patterns to capture the delimiter type and pass it through parsing logic.
Copilot uses AI. Check for mistakes.
public class TwoPartFormatParser : IFormatParser | ||
{ | ||
private static readonly Regex _beginRegex = new(@"^\s*", RegexOptions.Compiled); | ||
private static readonly Regex _beginRegex = new(@"^\s*(?:[\[\{])?\s*", RegexOptions.Compiled); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These patterns allow mismatches like an opening '{' with a closing ']' or a closing bracket without any opener, which can mask malformed input. Consider validating for a balanced, matching pair (and rejecting lone or mismatched delimiters) by deferring closing delimiter consumption until after successful two-part parsing and explicitly checking the original characters.
Copilot uses AI. Check for mistakes.
private static readonly Regex _beginRegex = new(@"^\s*(?:[\[\{])?\s*", RegexOptions.Compiled); | ||
private static readonly Regex _delimiterRegex = new(@"\G(?:\s*-\s*|\s+TO\s+)", RegexOptions.Compiled | RegexOptions.IgnoreCase); | ||
private static readonly Regex _endRegex = new(@"\G\s*$", RegexOptions.Compiled); | ||
private static readonly Regex _endRegex = new(@"\G\s*(?:[\]\}])?\s*$", RegexOptions.Compiled); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These patterns allow mismatches like an opening '{' with a closing ']' or a closing bracket without any opener, which can mask malformed input. Consider validating for a balanced, matching pair (and rejecting lone or mismatched delimiters) by deferring closing delimiter consumption until after successful two-part parsing and explicitly checking the original characters.
Copilot uses AI. Check for mistakes.
// Bracket notation support [start TO end] | ||
var bracketRange = DateTimeRange.Parse("[2023-01-01 TO 2023-12-31]", DateTime.Now); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Documentation shows only square bracket usage but the tests and parser also accept curly braces; the README does not clarify (a) whether {} are supported, or (b) inclusive vs exclusive semantics. Either document that {} are currently treated the same as [] (temporary limitation) or explain exclusivity once implemented to avoid developer confusion.
// Bracket notation support [start TO end] | |
var bracketRange = DateTimeRange.Parse("[2023-01-01 TO 2023-12-31]", DateTime.Now); | |
// Bracket notation support [start TO end] or {start TO end} | |
// Both square brackets [ ] and curly braces { } are accepted for range expressions. | |
// Currently, both are treated the same (inclusive bounds). Exclusive semantics for { } may be added in the future. | |
var bracketRange = DateTimeRange.Parse("[2023-01-01 TO 2023-12-31]", DateTime.Now); | |
var curlyBracketRange = DateTimeRange.Parse("{2023-01-01 TO 2023-12-31}", DateTime.Now); // Currently same as above |
Copilot uses AI. Check for mistakes.
case 4: // Built-in: year (yyyy) | ||
return TryParseWithFormat(dateStr, "yyyy", offset, false, out result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parser supports year-only and year-month anchors, but there are no corresponding tests exercising these formats (e.g., "2023||" or "2023-06||"). Add tests to ensure these branches remain correct and to catch regressions in length-based parsing.
Copilot generated this review using guidance from repository custom instructions.
case 7: // Built-in: year_month (yyyy-MM) | ||
if (dateStr[4] == '-') | ||
return TryParseWithFormat(dateStr, "yyyy-MM", offset, false, out result); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The parser supports year-only and year-month anchors, but there are no corresponding tests exercising these formats (e.g., "2023||" or "2023-06||"). Add tests to ensure these branches remain correct and to catch regressions in length-based parsing.
Copilot generated this review using guidance from repository custom instructions.
private static readonly Regex _parser = new( | ||
@"^(?<anchor>now|(?<date>\d{4}[-.]?\d{2}[-.]?\d{2}(?:[T\s](?:\d{1,2}(?::?\d{2}(?::?\d{2})?)?(?:\.\d{1,3})?)?(?:[+-]\d{2}:?\d{2}|Z)?)?)\|\|)" + | ||
@"(?<operations>(?:[+\-/]\d*[yMwdhHms])*)$", | ||
RegexOptions.Compiled | RegexOptions.IgnoreCase); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The regex permits multiple rounding operations or additional arithmetic after a rounding (e.g., "now/d+1h"), but tests do not cover invalid sequencing; Elasticsearch requires rounding (/unit) be the final operation. Add negative tests (expecting null) for sequences like "now/d+1h" and "/d/d" to ensure spec-aligned enforcement (or clarify in docs if divergence is intentional).
Copilot generated this review using guidance from repository custom instructions.
private static readonly Regex _parser = new( | ||
@"^(?<anchor>now|(?<date>\d{4}[-.]?\d{2}[-.]?\d{2}(?:[T\s](?:\d{1,2}(?::?\d{2}(?::?\d{2})?)?(?:\.\d{1,3})?)?(?:[+-]\d{2}:?\d{2}|Z)?)?)\|\|)" + |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The date portion allows a dotted format (e.g., 2001.02.01) via the [-.]?
separators, but there are no tests covering dotted date inputs. Add tests like "2001.02.01||" (with and without operations) to validate this branch and prevent future regressions.
Copilot generated this review using guidance from repository custom instructions.
Adds a `DateMath` utility class for parsing Elasticsearch date math expressions, offering standalone date math functionality without range capabilities. This utility supports parsing expressions with `now`, explicit dates, operations, and time units, providing a simpler API for direct parsing operations. Includes comprehensive unit tests for various parsing scenarios, edge cases, and timezone handling.
- Improves date parsing by removing support for dotted date formats and enforcing hyphenated formats for consistency. - Adds validation to ensure rounding operations in date math expressions are only used as the final operation, aligning with specification requirements. - Enhances two-part format parser to validate matching brackets, preventing parsing errors due to unbalanced brackets.
Adds validation to ensure that brackets and braces are properly matched in the format parser. This prevents incorrect parsing when there are mismatched brackets/braces.
Extends the DateMath utility to support TimeZoneInfo, enabling accurate date parsing and calculations within specific timezones. This enhancement allows for parsing expressions using a specified timezone, ensuring that "now" calculations and dates without explicit timezone information are correctly interpreted. Dates with explicit timezone information are preserved, regardless of the TimeZoneInfo parameter.
Updates test method names to clearly indicate that they specifically test timezone parsing functionality. This improves clarity and maintainability of the test suite.
https://www.elastic.co/docs/reference/query-languages/query-dsl/query-dsl-query-string-query#_ranges