-
Notifications
You must be signed in to change notification settings - Fork 1.5k
$dateToString aggregation with timezone outputs invalid ISO8601 string #1768
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
Changes from all commits
281c7a3
9b6b7a3
f4f59bd
0954ac5
3a4b76b
8f68b79
542c871
f64a01c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |
| import java.util.function.BinaryOperator; | ||
| import java.util.function.Function; | ||
|
|
||
| import static com.mongodb.assertions.Assertions.fail; | ||
| import static com.mongodb.client.model.mql.MqlValues.of; | ||
| import static com.mongodb.client.model.mql.MqlValues.ofNull; | ||
| import static com.mongodb.client.model.mql.MqlValues.ofStringArray; | ||
|
|
@@ -953,6 +954,12 @@ public MqlString asString(final MqlString timezone, final MqlString format) { | |
| .append("timezone", toBsonValue(cr, timezone)))); | ||
| } | ||
|
|
||
| public MqlString asString(final MqlString timezone) { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I believe that we do not need this method, and that it might actually be introducing the bug described in the Jira. That is, the API as it is now intentionally prevents the user from specifying a timezone without also forcing that user to explicitly specify a format, thus preventing the situation from arising. For reference:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thanks for sharing. Yeah, also saw you raising the point in our team slack channel. I have no objection of closing this PR. |
||
| // Given that server versions < 7.1 return a wrong format, not implementing this method helps prevent users | ||
| // from encountering the bug, described in DRIVERS-2620, by avoiding the method that requires a format as a parameter. | ||
| throw fail(); | ||
| } | ||
|
|
||
| @Override | ||
| public MqlDate parseDate(final MqlString timezone, final MqlString format) { | ||
| Assertions.notNull("timezone", timezone); | ||
|
|
||