cgivre commented on code in PR #2566:
URL: https://github.com/apache/drill/pull/2566#discussion_r883898280
##########
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:
@vdiravka Thanks for this PR. Will the options here be the global JSON
options defined in the Drill config? IE: If `allTextMode` is set globally,
will that be passed down to the UDF?
##########
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:
Here I think is a little different. The endpoint may define `jsonOptions`
that might differ from the global options. Can we pass that as well? I think
there's a method actually in the http config to actually get that object.
--
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: dev-unsubscr...@drill.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
