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

8.15.6 - ExtendedBounds -> ExtendedBoundsDate needs implicit convertors for anchored DateTime #8338

Open
Tracked by #8356
niemyjski opened this issue Sep 5, 2024 · 3 comments
Labels

Comments

@niemyjski
Copy link
Contributor

Elastic.Clients.Elasticsearch version: 8.15.6

Elasticsearch version: 8.15.1

.NET runtime version: 8.x

Operating system version: Any

Description of the problem including expected versus actual behavior:

We had the following code which is somewhat straightforward to convert but the FieldDateMath type seem to have a huge gap when it comes to working with DateTimes. It's not intuitive what I should pass to new FieldDateMath().

var bounds = new ExtendedBounds<DateMath> { Minimum = start, Maximum = end}; // start and end are DateTime's
...new DateHistogramAggregation { ExtendedBounds = bounds };

Expected behavior

Should be able to quickly do a bounds using dates.

Reference: FoundatioFx/Foundatio.Parsers#84

@flobernd
Copy link
Member

flobernd commented Sep 6, 2024

Hi @niemyjski,

this is actually a bug in the code generator. FieldDateMath is defined as:

export type FieldDateMath = DateMath | double

The correct output declaration should be:

public sealed partial class FieldDateMath : Union<DateMath, double>
{
    public FieldDateMath(DateMath expr) : base(expr) { }

    public FieldDateMath(double value) : base(value) { }
}

^ This would at least allow you to initialize the FieldDateMath with a DateTime value that is implicitly converted to DateMath. We can additionally add an implicit conversion operator from DateTime -> FieldDateMath in addition.

Currently, DateMath (which is an alias for string) incorrectly gets flattened to string:

public sealed partial class FieldDateMath : Union<string, double>
{
    public FieldDateMath(string Expr) : base(Expr) { }

    public FieldDateMath(double Value) : base(Value) { }
}

@niemyjski
Copy link
Contributor Author

Thanks! That would be great!

@niemyjski
Copy link
Contributor Author

Any idea when the code gen will be updated? I saw lots of code gen improvements in latest releases but this is still an issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants