[
https://issues.apache.org/jira/browse/DRILL-8236?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17543101#comment-17543101
]
ASF GitHub Bot commented on DRILL-8236:
---------------------------------------
vdiravka commented on code in PR #2566:
URL: https://github.com/apache/drill/pull/2566#discussion_r883941032
##########
contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/udfs/HttpHelperFunctions.java:
##########
@@ -52,22 +52,16 @@ public static class HttpGetFunction implements
DrillSimpleFunc {
OptionManager options;
@Inject
- DrillBuf buffer;
+ ResultSetLoader loader;
@Workspace
- org.apache.drill.exec.vector.complex.fn.JsonReader jsonReader;
+
org.apache.drill.exec.store.easy.json.loader.JsonLoaderImpl.JsonLoaderBuilder
jsonLoaderBuilder;
@Override
public void setup() {
- jsonReader = new
org.apache.drill.exec.vector.complex.fn.JsonReader.Builder(buffer)
- .defaultSchemaPathColumns()
-
.readNumbersAsDouble(options.getOption(org.apache.drill.exec.ExecConstants.JSON_READ_NUMBERS_AS_DOUBLE).bool_val)
-
.allTextMode(options.getOption(org.apache.drill.exec.ExecConstants.JSON_ALL_TEXT_MODE).bool_val)
-
.enableNanInf(options.getOption(org.apache.drill.exec.ExecConstants.JSON_READER_NAN_INF_NUMBERS).bool_val)
- .build();
-
- jsonReader.setIgnoreJSONParseErrors(
-
options.getBoolean(org.apache.drill.exec.ExecConstants.JSON_READER_SKIP_INVALID_RECORDS_FLAG));
+ jsonLoaderBuilder = new
org.apache.drill.exec.store.easy.json.loader.JsonLoaderImpl.JsonLoaderBuilder()
+ .resultSetLoader(loader)
+ .standardOptions(options);
Review Comment:
Yes. You can check, `JsonLoaderOptions` initialized from `OptionSet`:
```
public JsonLoaderBuilder standardOptions(OptionSet optionSet) {
this.options = new JsonLoaderOptions(optionSet);
return this;
}
```
And I have checked and can confirm - `ExecConstants.JSON_ALL_TEXT_MODE` sets
[JsonLoaderOptions.allTextMode](https://github.com/apache/drill/blob/master/exec/java-exec/src/main/java/org/apache/drill/exec/store/easy/json/loader/JsonLoaderOptions.java#L87)
##########
contrib/storage-http/src/main/java/org/apache/drill/exec/store/http/udfs/HttpHelperFunctions.java:
##########
@@ -147,15 +142,9 @@ public static class HttpGetFromStoragePluginFunction
implements DrillSimpleFunc
@Override
public void setup() {
- jsonReader = new
org.apache.drill.exec.vector.complex.fn.JsonReader.Builder(buffer)
- .defaultSchemaPathColumns()
-
.readNumbersAsDouble(options.getOption(org.apache.drill.exec.ExecConstants.JSON_READ_NUMBERS_AS_DOUBLE).bool_val)
-
.allTextMode(options.getOption(org.apache.drill.exec.ExecConstants.JSON_ALL_TEXT_MODE).bool_val)
-
.enableNanInf(options.getOption(org.apache.drill.exec.ExecConstants.JSON_READER_NAN_INF_NUMBERS).bool_val)
- .build();
-
- jsonReader.setIgnoreJSONParseErrors(
-
options.getBoolean(org.apache.drill.exec.ExecConstants.JSON_READER_SKIP_INVALID_RECORDS_FLAG));
+ jsonLoaderBuilder = new
org.apache.drill.exec.store.easy.json.loader.JsonLoaderImpl.JsonLoaderBuilder()
+ .resultSetLoader(loader)
+ .standardOptions(options);
Review Comment:
Answered in above comment.
> You can check, JsonLoaderOptions initialized from OptionSet:
```
public JsonLoaderBuilder standardOptions(OptionSet optionSet) {
this.options = new JsonLoaderOptions(optionSet);
return this;
}
```
> Move HttpHelperFunctions to use JSON2 reader
> --------------------------------------------
>
> Key: DRILL-8236
> URL: https://issues.apache.org/jira/browse/DRILL-8236
> Project: Apache Drill
> Issue Type: Improvement
> Components: Client - HTTP
> Affects Versions: 2.0.0
> Reporter: Vitalii Diravka
> Assignee: Vitalii Diravka
> Priority: Major
> Fix For: 2.0.0
>
>
> HttpHelperFunctions still uses old JSON reader. Need to swtich it to the new
> EVF based reader
--
This message was sent by Atlassian Jira
(v8.20.7#820007)
