Skip to content

Commit c8566b4

Browse files
duxiao1212duxiao1212
authored
revert json_extract to Produce Canonicalized Output (prestodb#24864)
1 parent 940a92a commit c8566b4

File tree

13 files changed

+164
-826
lines changed

13 files changed

+164
-826
lines changed

‎presto-common/src/main/java/com/facebook/presto/common/function/SqlFunctionProperties.java‎

Lines changed: 4 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,6 @@ public class SqlFunctionProperties
3737
private final boolean legacyJsonCast;
3838
private final Map<String, String> extraCredentials;
3939
private final boolean warnOnCommonNanPatterns;
40-
private final boolean canonicalizedJsonExtract;
4140

4241
private SqlFunctionProperties(
4342
boolean parseDecimalLiteralAsDouble,
@@ -51,8 +50,7 @@ private SqlFunctionProperties(
5150
boolean fieldNamesInJsonCastEnabled,
5251
boolean legacyJsonCast,
5352
Map<String, String> extraCredentials,
54-
boolean warnOnCommonNanPatterns,
55-
boolean canonicalizedJsonExtract)
53+
boolean warnOnCommonNanPatterns)
5654
{
5755
this.parseDecimalLiteralAsDouble = parseDecimalLiteralAsDouble;
5856
this.legacyRowFieldOrdinalAccessEnabled = legacyRowFieldOrdinalAccessEnabled;
@@ -66,7 +64,6 @@ private SqlFunctionProperties(
6664
this.legacyJsonCast = legacyJsonCast;
6765
this.extraCredentials = requireNonNull(extraCredentials, "extraCredentials is null");
6866
this.warnOnCommonNanPatterns = warnOnCommonNanPatterns;
69-
this.canonicalizedJsonExtract = canonicalizedJsonExtract;
7067
}
7168

7269
public boolean isParseDecimalLiteralAsDouble()
@@ -130,9 +127,6 @@ public boolean shouldWarnOnCommonNanPatterns()
130127
return warnOnCommonNanPatterns;
131128
}
132129

133-
public boolean isCanonicalizedJsonExtract()
134-
{ return canonicalizedJsonExtract; }
135-
136130
@Override
137131
public boolean equals(Object o)
138132
{
@@ -152,16 +146,15 @@ public boolean equals(Object o)
152146
Objects.equals(sessionLocale, that.sessionLocale) &&
153147
Objects.equals(sessionUser, that.sessionUser) &&
154148
Objects.equals(extraCredentials, that.extraCredentials) &&
155-
Objects.equals(legacyJsonCast, that.legacyJsonCast) &&
156-
Objects.equals(canonicalizedJsonExtract, that.legacyJsonCast);
149+
Objects.equals(legacyJsonCast, that.legacyJsonCast);
157150
}
158151

159152
@Override
160153
public int hashCode()
161154
{
162155
return Objects.hash(parseDecimalLiteralAsDouble, legacyRowFieldOrdinalAccessEnabled, timeZoneKey,
163156
legacyTimestamp, legacyMapSubscript, sessionStartTime, sessionLocale, sessionUser,
164-
extraCredentials, legacyJsonCast, canonicalizedJsonExtract);
157+
extraCredentials, legacyJsonCast);
165158
}
166159

167160
public static Builder builder()
@@ -183,7 +176,6 @@ public static class Builder
183176
private boolean legacyJsonCast;
184177
private Map<String, String> extraCredentials = emptyMap();
185178
private boolean warnOnCommonNanPatterns;
186-
private boolean canonicalizedJsonExtract;
187179

188180
private Builder() {}
189181

@@ -259,12 +251,6 @@ public Builder setWarnOnCommonNanPatterns(boolean warnOnCommonNanPatterns)
259251
return this;
260252
}
261253

262-
public Builder setCanonicalizedJsonExtract(boolean canonicalizedJsonExtract)
263-
{
264-
this.canonicalizedJsonExtract = canonicalizedJsonExtract;
265-
return this;
266-
}
267-
268254
public SqlFunctionProperties build()
269255
{
270256
return new SqlFunctionProperties(
@@ -279,8 +265,7 @@ public SqlFunctionProperties build()
279265
fieldNamesInJsonCastEnabled,
280266
legacyJsonCast,
281267
extraCredentials,
282-
warnOnCommonNanPatterns,
283-
canonicalizedJsonExtract);
268+
warnOnCommonNanPatterns);
284269
}
285270
}
286271
}

‎presto-main-base/src/main/java/com/facebook/presto/Session.java‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@
5757
import java.util.stream.Collectors;
5858

5959
import static com.facebook.presto.SystemSessionProperties.LEGACY_JSON_CAST;
60-
import static com.facebook.presto.SystemSessionProperties.isCanonicalizedJsonExtract;
6160
import static com.facebook.presto.SystemSessionProperties.isFieldNameInJsonCastEnabled;
6261
import static com.facebook.presto.SystemSessionProperties.isLegacyMapSubscript;
6362
import static com.facebook.presto.SystemSessionProperties.isLegacyRowFieldOrdinalAccessEnabled;
@@ -482,7 +481,6 @@ public SqlFunctionProperties getSqlFunctionProperties()
482481
.setLegacyJsonCast(legacyJsonCast)
483482
.setExtraCredentials(identity.getExtraCredentials())
484483
.setWarnOnCommonNanPatterns(warnOnCommonNanPatterns(this))
485-
.setCanonicalizedJsonExtract(isCanonicalizedJsonExtract(this))
486484
.build();
487485
}
488486

‎presto-main-base/src/main/java/com/facebook/presto/SystemSessionProperties.java‎

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -305,7 +305,6 @@ public final class SystemSessionProperties
305305
public static final String REWRITE_CASE_TO_MAP_ENABLED = "rewrite_case_to_map_enabled";
306306
public static final String FIELD_NAMES_IN_JSON_CAST_ENABLED = "field_names_in_json_cast_enabled";
307307
public static final String LEGACY_JSON_CAST = "legacy_json_cast";
308-
public static final String CANONICALIZED_JSON_EXTRACT = "canonicalized_json_extract";
309308
public static final String PULL_EXPRESSION_FROM_LAMBDA_ENABLED = "pull_expression_from_lambda_enabled";
310309
public static final String REWRITE_CONSTANT_ARRAY_CONTAINS_TO_IN_EXPRESSION = "rewrite_constant_array_contains_to_in_expression";
311310
public static final String INFER_INEQUALITY_PREDICATES = "infer_inequality_predicates";
@@ -1637,11 +1636,6 @@ public SystemSessionProperties(
16371636
"Keep the legacy json cast behavior, do not reserve the case for field names when casting to row type",
16381637
functionsConfig.isLegacyJsonCast(),
16391638
true),
1640-
booleanProperty(
1641-
CANONICALIZED_JSON_EXTRACT,
1642-
"Extracts json data in a canonicalized manner, and raises a PrestoException when encountering invalid json structures within the input json path",
1643-
functionsConfig.isCanonicalizedJsonExtract(),
1644-
true),
16451639
booleanProperty(
16461640
OPTIMIZE_JOIN_PROBE_FOR_EMPTY_BUILD_RUNTIME,
16471641
"Optimize join probe at runtime if build side is empty",
@@ -3180,9 +3174,4 @@ public static boolean isEnabledAddExchangeBelowGroupId(Session session)
31803174
{
31813175
return session.getSystemProperty(ADD_EXCHANGE_BELOW_PARTIAL_AGGREGATION_OVER_GROUP_ID, Boolean.class);
31823176
}
3183-
3184-
public static boolean isCanonicalizedJsonExtract(Session session)
3185-
{
3186-
return session.getSystemProperty(CANONICALIZED_JSON_EXTRACT, Boolean.class);
3187-
}
31883177
}

‎presto-main-base/src/main/java/com/facebook/presto/operator/scalar/JsonExtract.java‎

Lines changed: 16 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -13,23 +13,19 @@
1313
*/
1414
package com.facebook.presto.operator.scalar;
1515

16-
import com.facebook.airlift.json.JsonObjectMapperProvider;
17-
import com.facebook.presto.common.function.SqlFunctionProperties;
1816
import com.facebook.presto.spi.PrestoException;
1917
import com.fasterxml.jackson.core.JsonFactory;
2018
import com.fasterxml.jackson.core.JsonGenerator;
2119
import com.fasterxml.jackson.core.JsonParseException;
2220
import com.fasterxml.jackson.core.JsonParser;
2321
import com.fasterxml.jackson.core.JsonToken;
2422
import com.fasterxml.jackson.core.io.SerializedString;
25-
import com.fasterxml.jackson.databind.ObjectMapper;
2623
import com.google.common.collect.ImmutableList;
2724
import io.airlift.slice.DynamicSliceOutput;
2825
import io.airlift.slice.Slice;
2926

3027
import java.io.IOException;
3128
import java.io.InputStream;
32-
import java.io.OutputStream;
3329
import java.io.UncheckedIOException;
3430

3531
import static com.facebook.presto.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
@@ -42,7 +38,6 @@
4238
import static com.fasterxml.jackson.core.JsonToken.START_ARRAY;
4339
import static com.fasterxml.jackson.core.JsonToken.START_OBJECT;
4440
import static com.fasterxml.jackson.core.JsonToken.VALUE_NULL;
45-
import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS;
4641
import static io.airlift.slice.Slices.utf8Slice;
4742
import static java.util.Objects.requireNonNull;
4843

@@ -126,15 +121,13 @@ public final class JsonExtract
126121
private static final JsonFactory JSON_FACTORY = new JsonFactory()
127122
.disable(CANONICALIZE_FIELD_NAMES);
128123

129-
private static final ObjectMapper SORTED_MAPPER = new JsonObjectMapperProvider().get().configure(ORDER_MAP_ENTRIES_BY_KEYS, true);
130-
131124
private JsonExtract() {}
132125

133-
public static <T> T extract(Slice jsonInput, JsonExtractor<T> jsonExtractor, SqlFunctionProperties properties)
126+
public static <T> T extract(Slice jsonInput, JsonExtractor<T> jsonExtractor)
134127
{
135128
requireNonNull(jsonInput, "jsonInput is null");
136129
try {
137-
return jsonExtractor.extract(jsonInput.getInput(), properties);
130+
return jsonExtractor.extract(jsonInput.getInput());
138131
}
139132
catch (JsonParseException e) {
140133
// Return null if we failed to parse something
@@ -163,7 +156,7 @@ public static <T> PrestoJsonExtractor<T> generateExtractor(String path, PrestoJs
163156

164157
public interface JsonExtractor<T>
165158
{
166-
T extract(InputStream inputStream, SqlFunctionProperties properties)
159+
T extract(InputStream inputStream)
167160
throws IOException;
168161
}
169162

@@ -181,11 +174,11 @@ public abstract static class PrestoJsonExtractor<T>
181174
*
182175
* @return the value, or null if not applicable
183176
*/
184-
abstract T extract(JsonParser jsonParser, SqlFunctionProperties properties)
177+
abstract T extract(JsonParser jsonParser)
185178
throws IOException;
186179

187180
@Override
188-
public T extract(InputStream inputStream, SqlFunctionProperties properties)
181+
public T extract(InputStream inputStream)
189182
throws IOException
190183
{
191184
try (JsonParser jsonParser = createJsonParser(JSON_FACTORY, inputStream)) {
@@ -194,7 +187,7 @@ public T extract(InputStream inputStream, SqlFunctionProperties properties)
194187
return null;
195188
}
196189

197-
return extract(jsonParser, properties);
190+
return extract(jsonParser);
198191
}
199192
}
200193
}
@@ -221,21 +214,21 @@ public ObjectFieldJsonExtractor(String fieldName, PrestoJsonExtractor<? extends
221214
}
222215

223216
@Override
224-
public T extract(JsonParser jsonParser, SqlFunctionProperties properties)
217+
public T extract(JsonParser jsonParser)
225218
throws IOException
226219
{
227220
if (jsonParser.getCurrentToken() == START_OBJECT) {
228-
return processJsonObject(jsonParser, properties);
221+
return processJsonObject(jsonParser);
229222
}
230223

231224
if (jsonParser.getCurrentToken() == START_ARRAY) {
232-
return processJsonArray(jsonParser, properties);
225+
return processJsonArray(jsonParser);
233226
}
234227

235228
throw new JsonParseException(jsonParser, "Expected a JSON object or array");
236229
}
237230

238-
public T processJsonObject(JsonParser jsonParser, SqlFunctionProperties properties)
231+
public T processJsonObject(JsonParser jsonParser)
239232
throws IOException
240233
{
241234
while (!jsonParser.nextFieldName(fieldName)) {
@@ -251,10 +244,10 @@ public T processJsonObject(JsonParser jsonParser, SqlFunctionProperties properti
251244

252245
jsonParser.nextToken(); // Shift to first token of the value
253246

254-
return delegate.extract(jsonParser, properties);
247+
return delegate.extract(jsonParser);
255248
}
256249

257-
public T processJsonArray(JsonParser jsonParser, SqlFunctionProperties properties)
250+
public T processJsonArray(JsonParser jsonParser)
258251
throws IOException
259252
{
260253
int currentIndex = 0;
@@ -277,15 +270,15 @@ public T processJsonArray(JsonParser jsonParser, SqlFunctionProperties propertie
277270
jsonParser.skipChildren(); // Skip nested structure if currently at the start of one
278271
}
279272

280-
return delegate.extract(jsonParser, properties);
273+
return delegate.extract(jsonParser);
281274
}
282275
}
283276

284277
public static class ScalarValueJsonExtractor
285278
extends PrestoJsonExtractor<Slice>
286279
{
287280
@Override
288-
public Slice extract(JsonParser jsonParser, SqlFunctionProperties properties)
281+
public Slice extract(JsonParser jsonParser)
289282
throws IOException
290283
{
291284
JsonToken token = jsonParser.getCurrentToken();
@@ -303,31 +296,13 @@ public static class JsonValueJsonExtractor
303296
extends PrestoJsonExtractor<Slice>
304297
{
305298
@Override
306-
public Slice extract(JsonParser jsonParser, SqlFunctionProperties properties)
299+
public Slice extract(JsonParser jsonParser)
307300
throws IOException
308301
{
309302
if (!jsonParser.hasCurrentToken()) {
310303
throw new JsonParseException(jsonParser, "Unexpected end of value");
311304
}
312-
if (!properties.isCanonicalizedJsonExtract()) {
313-
return legacyExtract(jsonParser);
314-
}
315-
DynamicSliceOutput dynamicSliceOutput = new DynamicSliceOutput(ESTIMATED_JSON_OUTPUT_SIZE);
316-
// Write the JSON to output stream with sorted keys
317-
SORTED_MAPPER.writeValue((OutputStream) dynamicSliceOutput, SORTED_MAPPER.readValue(jsonParser, Object.class));
318-
// nextToken will throw an exception if there are trailing characters.
319-
try {
320-
jsonParser.nextToken();
321-
}
322-
catch (JsonParseException e) {
323-
throw new PrestoException(INVALID_FUNCTION_ARGUMENT, e.getMessage());
324-
}
325-
return dynamicSliceOutput.slice();
326-
}
327305

328-
public Slice legacyExtract(JsonParser jsonParser)
329-
throws IOException
330-
{
331306
DynamicSliceOutput dynamicSliceOutput = new DynamicSliceOutput(ESTIMATED_JSON_OUTPUT_SIZE);
332307
try (JsonGenerator jsonGenerator = createJsonGenerator(JSON_FACTORY, dynamicSliceOutput)) {
333308
jsonGenerator.copyCurrentStructure(jsonParser);
@@ -340,7 +315,7 @@ public static class JsonSizeExtractor
340315
extends PrestoJsonExtractor<Long>
341316
{
342317
@Override
343-
public Long extract(JsonParser jsonParser, SqlFunctionProperties properties)
318+
public Long extract(JsonParser jsonParser)
344319
throws IOException
345320
{
346321
if (!jsonParser.hasCurrentToken()) {

‎presto-main-base/src/main/java/com/facebook/presto/operator/scalar/JsonFunctions.java‎

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -435,51 +435,51 @@ public static Slice jsonArrayGet(@SqlType(StandardTypes.JSON) Slice json, @SqlTy
435435
@SqlNullable
436436
@LiteralParameters("x")
437437
@SqlType("varchar(x)")
438-
public static Slice varcharJsonExtractScalar(SqlFunctionProperties properties, @SqlType("varchar(x)") Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
438+
public static Slice varcharJsonExtractScalar(@SqlType("varchar(x)") Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
439439
{
440-
return JsonExtract.extract(json, jsonPath.getScalarExtractor(), properties);
440+
return JsonExtract.extract(json, jsonPath.getScalarExtractor());
441441
}
442442

443443
@ScalarFunction
444444
@SqlNullable
445445
@SqlType(StandardTypes.VARCHAR)
446-
public static Slice jsonExtractScalar(SqlFunctionProperties properties, @SqlType(StandardTypes.JSON) Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
446+
public static Slice jsonExtractScalar(@SqlType(StandardTypes.JSON) Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
447447
{
448-
return JsonExtract.extract(json, jsonPath.getScalarExtractor(), properties);
448+
return JsonExtract.extract(json, jsonPath.getScalarExtractor());
449449
}
450450

451451
@ScalarFunction("json_extract")
452452
@LiteralParameters("x")
453453
@SqlNullable
454454
@SqlType(StandardTypes.JSON)
455-
public static Slice varcharJsonExtract(SqlFunctionProperties properties, @SqlType("varchar(x)") Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
455+
public static Slice varcharJsonExtract(@SqlType("varchar(x)") Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
456456
{
457-
return JsonExtract.extract(json, jsonPath.getObjectExtractor(), properties);
457+
return JsonExtract.extract(json, jsonPath.getObjectExtractor());
458458
}
459459

460460
@ScalarFunction
461461
@SqlNullable
462462
@SqlType(StandardTypes.JSON)
463-
public static Slice jsonExtract(SqlFunctionProperties properties, @SqlType(StandardTypes.JSON) Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
463+
public static Slice jsonExtract(@SqlType(StandardTypes.JSON) Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
464464
{
465-
return JsonExtract.extract(json, jsonPath.getObjectExtractor(), properties);
465+
return JsonExtract.extract(json, jsonPath.getObjectExtractor());
466466
}
467467

468468
@ScalarFunction("json_size")
469469
@LiteralParameters("x")
470470
@SqlNullable
471471
@SqlType(StandardTypes.BIGINT)
472-
public static Long varcharJsonSize(SqlFunctionProperties properties, @SqlType("varchar(x)") Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
472+
public static Long varcharJsonSize(@SqlType("varchar(x)") Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
473473
{
474-
return JsonExtract.extract(json, jsonPath.getSizeExtractor(), properties);
474+
return JsonExtract.extract(json, jsonPath.getSizeExtractor());
475475
}
476476

477477
@ScalarFunction
478478
@SqlNullable
479479
@SqlType(StandardTypes.BIGINT)
480-
public static Long jsonSize(SqlFunctionProperties properties, @SqlType(StandardTypes.JSON) Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
480+
public static Long jsonSize(@SqlType(StandardTypes.JSON) Slice json, @SqlType(JsonPathType.NAME) JsonPath jsonPath)
481481
{
482-
return JsonExtract.extract(json, jsonPath.getSizeExtractor(), properties);
482+
return JsonExtract.extract(json, jsonPath.getSizeExtractor());
483483
}
484484

485485
public static Object getJsonObjectValue(Type valueType, SqlFunctionProperties properties, Block block, int position)

0 commit comments

Comments
 (0)