kmcginnes commented on code in PR #3090:
URL: https://github.com/apache/tinkerpop/pull/3090#discussion_r2029497263
##########
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/translator/JavascriptTranslateVisitor.java:
##########
@@ -133,9 +134,9 @@ public Void visitMapEntry(final
GremlinParser.MapEntryContext ctx) {
public Void visitDateLiteral(final GremlinParser.DateLiteralContext ctx) {
// child at 2 is the date argument to datetime() and comes enclosed in
quotes
final String dtString = ctx.getChild(2).getText();
- final Date dt =
DatetimeHelper.parse(removeFirstAndLastCharacters(dtString));
+ final OffsetDateTime dt =
DatetimeHelper.parse(removeFirstAndLastCharacters(dtString));
sb.append("new Date(");
- sb.append(dt.getTime());
+ sb.append(dt.toInstant().toEpochMilli());
Review Comment:
This should work, and it seems equivalent to what was there before. As long
as you've tested this with various time zones it should be fine.
However, I'd wager that if someone is using the translator to translate a
query, I doubt they started with a date value as the millisecond since unix
epoch. More likely they started with a string version of the date time.
Perhaps a better approach would be to use the ISO string value as the input
to `new Date()`. The precision should be the same (milliseconds), and it
supports time zone offsets or UTC. It's just a more human friendly translation.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
