Skip to content

Commit 6998ea7

Browse files
matrivmatriv
committed
SQL: Fix issue with date columns returned always in UTC (#40163)
When selecting columns of ES type `date` (SQL's DATETIME) the `FieldHitExtractor` was not using the timezone of the client session but always resorted to UTC. The same behaviour (UTC only) was encountered also for grouping keys (`CompositeKeyExtractor`) and for First/Last functions on dates (`TopHitsAggExtractor`). Fixes: #40152
1 parent df1883d commit 6998ea7

File tree

17 files changed

+222
-165
lines changed

17 files changed

+222
-165
lines changed

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/JdbcTestUtils.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ public static void logLikeCLI(ResultSet rs, Logger logger) throws SQLException {
137137
logger.info("\n" + formatter.formatWithHeader(cols, data));
138138
}
139139

140-
public static String of(long millis) {
141-
return StringUtils.toString(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), UTC));
140+
public static String of(long millis, String zoneId) {
141+
return StringUtils.toString(ZonedDateTime.ofInstant(Instant.ofEpochMilli(millis), ZoneId.of(zoneId)));
142142
}
143-
}
143+
}

‎x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java

Lines changed: 56 additions & 46 deletions
Large diffs are not rendered by default.

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/Querier.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -360,7 +360,6 @@ static class CompositeActionListener extends BaseAggActionListener {
360360
super(listener, client, cfg, output, query, request);
361361
}
362362

363-
364363
@Override
365364
protected void handleResponse(SearchResponse response, ActionListener<SchemaRowSet> listener) {
366365
// there are some results
@@ -423,7 +422,7 @@ protected List<BucketExtractor> initBucketExtractors(SearchResponse response) {
423422
private BucketExtractor createExtractor(FieldExtraction ref, BucketExtractor totalCount) {
424423
if (ref instanceof GroupByRef) {
425424
GroupByRef r = (GroupByRef) ref;
426-
return new CompositeKeyExtractor(r.key(), r.property(), r.zoneId());
425+
return new CompositeKeyExtractor(r.key(), r.property(), cfg.zoneId(), r.isDateTimeBased());
427426
}
428427

429428
if (ref instanceof MetricAggRef) {
@@ -433,7 +432,7 @@ private BucketExtractor createExtractor(FieldExtraction ref, BucketExtractor tot
433432

434433
if (ref instanceof TopHitsAggRef) {
435434
TopHitsAggRef r = (TopHitsAggRef) ref;
436-
return new TopHitsAggExtractor(r.name(), r.fieldDataType());
435+
return new TopHitsAggExtractor(r.name(), r.fieldDataType(), cfg.zoneId());
437436
}
438437

439438
if (ref == GlobalCountRef.INSTANCE) {
@@ -513,12 +512,13 @@ protected void handleResponse(SearchResponse response, ActionListener<SchemaRowS
513512
private HitExtractor createExtractor(FieldExtraction ref) {
514513
if (ref instanceof SearchHitFieldRef) {
515514
SearchHitFieldRef f = (SearchHitFieldRef) ref;
516-
return new FieldHitExtractor(f.name(), f.getDataType(), f.useDocValue(), f.hitName(), multiValueFieldLeniency);
515+
return new FieldHitExtractor(f.name(), f.getDataType(), cfg.zoneId(),
516+
f.useDocValue(), f.hitName(), multiValueFieldLeniency);
517517
}
518518

519519
if (ref instanceof ScriptFieldRef) {
520520
ScriptFieldRef f = (ScriptFieldRef) ref;
521-
return new FieldHitExtractor(f.name(), null, true, multiValueFieldLeniency);
521+
return new FieldHitExtractor(f.name(), null, cfg.zoneId(), true, multiValueFieldLeniency);
522522
}
523523

524524
if (ref instanceof ComputedRef) {

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/CompositeKeyExtractor.java

Lines changed: 16 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -20,44 +20,38 @@
2020
public class CompositeKeyExtractor implements BucketExtractor {
2121

2222
/**
23-
* Key or Komposite extractor.
23+
* Key or Composite extractor.
2424
*/
2525
static final String NAME = "k";
2626

2727
private final String key;
2828
private final Property property;
2929
private final ZoneId zoneId;
30+
private final boolean isDateTimeBased;
3031

3132
/**
3233
* Constructs a new <code>CompositeKeyExtractor</code> instance.
33-
* The time-zone parameter is used to indicate a date key.
3434
*/
35-
public CompositeKeyExtractor(String key, Property property, ZoneId zoneId) {
35+
public CompositeKeyExtractor(String key, Property property, ZoneId zoneId, boolean isDateTimeBased) {
3636
this.key = key;
3737
this.property = property;
3838
this.zoneId = zoneId;
39+
this.isDateTimeBased = isDateTimeBased;
3940
}
4041

4142
CompositeKeyExtractor(StreamInput in) throws IOException {
4243
key = in.readString();
4344
property = in.readEnum(Property.class);
44-
if (in.readBoolean()) {
45-
zoneId = ZoneId.of(in.readString());
46-
} else {
47-
zoneId = null;
48-
}
45+
zoneId = ZoneId.of(in.readString());
46+
isDateTimeBased = in.readBoolean();
4947
}
5048

5149
@Override
5250
public void writeTo(StreamOutput out) throws IOException {
5351
out.writeString(key);
5452
out.writeEnum(property);
55-
if (zoneId == null) {
56-
out.writeBoolean(false);
57-
} else {
58-
out.writeBoolean(true);
59-
out.writeString(zoneId.getId());
60-
}
53+
out.writeString(zoneId.getId());
54+
out.writeBoolean(isDateTimeBased);
6155
}
6256

6357
String key() {
@@ -72,6 +66,10 @@ ZoneId zoneId() {
7266
return zoneId;
7367
}
7468

69+
public boolean isDateTimeBased() {
70+
return isDateTimeBased;
71+
}
72+
7573
@Override
7674
public String getWriteableName() {
7775
return NAME;
@@ -91,7 +89,7 @@ public Object extract(Bucket bucket) {
9189

9290
Object object = ((Map<?, ?>) m).get(key);
9391

94-
if (zoneId != null) {
92+
if (isDateTimeBased) {
9593
if (object == null) {
9694
return object;
9795
} else if (object instanceof Long) {
@@ -106,7 +104,7 @@ public Object extract(Bucket bucket) {
106104

107105
@Override
108106
public int hashCode() {
109-
return Objects.hash(key, property, zoneId);
107+
return Objects.hash(key, property, zoneId, isDateTimeBased);
110108
}
111109

112110
@Override
@@ -122,7 +120,8 @@ public boolean equals(Object obj) {
122120
CompositeKeyExtractor other = (CompositeKeyExtractor) obj;
123121
return Objects.equals(key, other.key)
124122
&& Objects.equals(property, other.property)
125-
&& Objects.equals(zoneId, other.zoneId);
123+
&& Objects.equals(zoneId, other.zoneId)
124+
&& Objects.equals(isDateTimeBased, other.isDateTimeBased);
126125
}
127126

128127
@Override

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/FieldHitExtractor.java

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.joda.time.DateTime;
1818

1919
import java.io.IOException;
20+
import java.time.ZoneId;
2021
import java.util.ArrayDeque;
2122
import java.util.Deque;
2223
import java.util.List;
@@ -46,21 +47,23 @@ private static String[] sourcePath(String name, boolean useDocValue, String hitN
4647

4748
private final String fieldName, hitName;
4849
private final DataType dataType;
50+
private final ZoneId zoneId;
4951
private final boolean useDocValue;
5052
private final boolean arrayLeniency;
5153
private final String[] path;
5254

53-
public FieldHitExtractor(String name, DataType dataType, boolean useDocValue) {
54-
this(name, dataType, useDocValue, null, false);
55+
public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue) {
56+
this(name, dataType, zoneId, useDocValue, null, false);
5557
}
5658

57-
public FieldHitExtractor(String name, DataType dataType, boolean useDocValue, boolean arrayLeniency) {
58-
this(name, dataType, useDocValue, null, arrayLeniency);
59+
public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue, boolean arrayLeniency) {
60+
this(name, dataType, zoneId, useDocValue, null, arrayLeniency);
5961
}
6062

61-
public FieldHitExtractor(String name, DataType dataType, boolean useDocValue, String hitName, boolean arrayLeniency) {
63+
public FieldHitExtractor(String name, DataType dataType, ZoneId zoneId, boolean useDocValue, String hitName, boolean arrayLeniency) {
6264
this.fieldName = name;
6365
this.dataType = dataType;
66+
this.zoneId = zoneId;
6467
this.useDocValue = useDocValue;
6568
this.arrayLeniency = arrayLeniency;
6669
this.hitName = hitName;
@@ -78,6 +81,7 @@ public FieldHitExtractor(String name, DataType dataType, boolean useDocValue, St
7881
fieldName = in.readString();
7982
String esType = in.readOptionalString();
8083
dataType = esType != null ? DataType.fromTypeName(esType) : null;
84+
zoneId = ZoneId.of(in.readString());
8185
useDocValue = in.readBoolean();
8286
hitName = in.readOptionalString();
8387
arrayLeniency = in.readBoolean();
@@ -93,6 +97,7 @@ public String getWriteableName() {
9397
public void writeTo(StreamOutput out) throws IOException {
9498
out.writeString(fieldName);
9599
out.writeOptionalString(dataType == null ? null : dataType.typeName);
100+
out.writeString(zoneId.getId());
96101
out.writeBoolean(useDocValue);
97102
out.writeOptionalString(hitName);
98103
out.writeBoolean(arrayLeniency);
@@ -136,7 +141,7 @@ private Object unwrapMultiValue(Object values) {
136141
}
137142
if (dataType == DataType.DATETIME) {
138143
if (values instanceof String) {
139-
return DateUtils.asDateTime(Long.parseLong(values.toString()));
144+
return DateUtils.asDateTime(Long.parseLong(values.toString()), zoneId);
140145
}
141146
// returned by nested types...
142147
if (values instanceof DateTime) {
@@ -220,9 +225,17 @@ public String fieldName() {
220225
return fieldName;
221226
}
222227

228+
public ZoneId zoneId() {
229+
return zoneId;
230+
}
231+
232+
DataType dataType() {
233+
return dataType;
234+
}
235+
223236
@Override
224237
public String toString() {
225-
return fieldName + "@" + hitName;
238+
return fieldName + "@" + hitName + "@" + zoneId;
226239
}
227240

228241
@Override

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/search/extractor/TopHitsAggExtractor.java

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import org.elasticsearch.xpack.sql.util.DateUtils;
1515

1616
import java.io.IOException;
17+
import java.time.ZoneId;
1718
import java.util.Objects;
1819

1920
public class TopHitsAggExtractor implements BucketExtractor {
@@ -22,27 +23,39 @@ public class TopHitsAggExtractor implements BucketExtractor {
2223

2324
private final String name;
2425
private final DataType fieldDataType;
26+
private final ZoneId zoneId;
2527

26-
public TopHitsAggExtractor(String name, DataType fieldDataType) {
28+
public TopHitsAggExtractor(String name, DataType fieldDataType, ZoneId zoneId) {
2729
this.name = name;
2830
this.fieldDataType = fieldDataType;
31+
this.zoneId = zoneId;
2932
}
3033

3134
TopHitsAggExtractor(StreamInput in) throws IOException {
3235
name = in.readString();
3336
fieldDataType = in.readEnum(DataType.class);
37+
zoneId = ZoneId.of(in.readString());
3438
}
3539

3640
@Override
3741
public void writeTo(StreamOutput out) throws IOException {
3842
out.writeString(name);
3943
out.writeEnum(fieldDataType);
44+
out.writeString(zoneId.getId());
4045
}
4146

4247
String name() {
4348
return name;
4449
}
4550

51+
DataType fieldDataType() {
52+
return fieldDataType;
53+
}
54+
55+
ZoneId zoneId() {
56+
return zoneId;
57+
}
58+
4659
@Override
4760
public String getWriteableName() {
4861
return NAME;
@@ -61,15 +74,15 @@ public Object extract(Bucket bucket) {
6174

6275
Object value = agg.getHits().getAt(0).getFields().values().iterator().next().getValue();
6376
if (fieldDataType.isDateBased()) {
64-
return DateUtils.asDateTime(Long.parseLong(value.toString()));
77+
return DateUtils.asDateTime(Long.parseLong(value.toString()), zoneId);
6578
} else {
6679
return value;
6780
}
6881
}
6982

7083
@Override
7184
public int hashCode() {
72-
return Objects.hash(name, fieldDataType);
85+
return Objects.hash(name, fieldDataType, zoneId);
7386
}
7487

7588
@Override
@@ -84,11 +97,12 @@ public boolean equals(Object obj) {
8497

8598
TopHitsAggExtractor other = (TopHitsAggExtractor) obj;
8699
return Objects.equals(name, other.name)
87-
&& Objects.equals(fieldDataType, other.fieldDataType);
100+
&& Objects.equals(fieldDataType, other.fieldDataType)
101+
&& Objects.equals(zoneId, other.zoneId);
88102
}
89103

90104
@Override
91105
public String toString() {
92-
return "TopHits>" + name + "[" + fieldDataType + "]";
106+
return "TopHits>" + name + "[" + fieldDataType + "]@" + zoneId;
93107
}
94108
}

‎x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/planner/Planner.java

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,14 @@
55
*/
66
package org.elasticsearch.xpack.sql.planner;
77

8-
import java.util.List;
9-
import java.util.Map;
10-
118
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
129
import org.elasticsearch.xpack.sql.plan.physical.PhysicalPlan;
1310
import org.elasticsearch.xpack.sql.planner.Verifier.Failure;
1411
import org.elasticsearch.xpack.sql.tree.Node;
1512

13+
import java.util.List;
14+
import java.util.Map;
15+
1616
import static java.util.stream.Collectors.toMap;
1717

1818
public class Planner {
@@ -64,4 +64,4 @@ public Map<Node<?>, String> verifyExecutingPlanFailures(PhysicalPlan plan) {
6464
List<Failure> failures = Verifier.verifyExecutingPlan(plan);
6565
return failures.stream().collect(toMap(Failure::source, Failure::message));
6666
}
67-
}
67+
}

0 commit comments

Comments
 (0)