[ 
https://issues.apache.org/jira/browse/DRILL-8474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17803594#comment-17803594
 ] 

ASF GitHub Bot commented on DRILL-8474:
---------------------------------------

mbeckerle commented on code in PR #2836:
URL: https://github.com/apache/drill/pull/2836#discussion_r1442993784


##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/schema/DrillDaffodilSchemaVisitor.java:
##########
@@ -0,0 +1,229 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.drill.exec.store.daffodil.schema;
+
+import org.apache.daffodil.runtime1.api.ChoiceMetadata;
+import org.apache.daffodil.runtime1.api.ComplexElementMetadata;
+import org.apache.daffodil.runtime1.api.ElementMetadata;
+import org.apache.daffodil.runtime1.api.InfosetSimpleElement;
+import org.apache.daffodil.runtime1.api.MetadataHandler;
+import org.apache.daffodil.runtime1.api.SequenceMetadata;
+import org.apache.daffodil.runtime1.api.SimpleElementMetadata;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.record.metadata.MapBuilder;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.util.Stack;
+
+/**
+ * This class transforms a DFDL/Daffodil schema into a Drill Schema.
+ */
+public class DrillDaffodilSchemaVisitor extends MetadataHandler {
+  private static final Logger logger = 
LoggerFactory.getLogger(DrillDaffodilSchemaVisitor.class);
+  /**
+   * Unfortunately, SchemaBuilder and MapBuilder, while similar, do not share 
a base class so we
+   * have a stack of MapBuilders, and when empty we use the SchemaBuilder

Review Comment:
   Note that this awkwardness effectively doubles the code size of things that 
interface to Drill. 
   
   This duplication of similar behavior for schema and map builders (and 
rowWriters and mapWriters) is expected and typical of systems that start from a 
tabular view of the data world and later add the features needed for 
hierachical data. Nevertheless it is awkward when one is dealing entirely with 
hierarchical data. 
   
   A MetaBuilder that does the map thing if the builder is a map, and the 
schema thing if the builder is a schema would eliminate this. This could be an 
interface mixed into both SchemaBuilder and MapBuilder (could also be called 
MapBuilderLike). 
   
   The same discontinuity at the base holds for RowWriter vs. MapWriter in the 
runtime handling of data. Again it doubles the code size/complexity, every fix 
goes in 2 places, etc. A MapWriterLike interface could be factored out.
   
   Maybe we should build such mechanisms to avoid this, and then use them to 
improve this Daffodil plugin?
   



##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/schema/DrillDaffodilSchemaUtils.java:
##########
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.drill.exec.store.daffodil.schema;
+
+import org.apache.daffodil.japi.InvalidParserException;
+import org.apache.daffodil.japi.DataProcessor;
+import org.apache.daffodil.runtime1.api.PrimitiveType;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.collect.ImmutableMap;
+
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+
+
+public class DrillDaffodilSchemaUtils {
+  private static final MinorType DEFAULT_TYPE = MinorType.VARCHAR;
+  private static final Logger logger = 
LoggerFactory.getLogger(DrillDaffodilSchemaUtils.class);
+
+  /**
+   * This map maps the data types defined by the DFDL definition to Drill data 
types.
+   */
+  public static final ImmutableMap<String, MinorType> DFDL_TYPE_MAPPINGS =
+      ImmutableMap.<String, MinorType>builder()
+          .put("LONG", MinorType.BIGINT)
+          .put("INT", MinorType.INT)
+          .put("SHORT", MinorType.SMALLINT)
+          .put("BYTE", MinorType.TINYINT)
+          // daffodil unsigned longs are modeled as DECIMAL(38, 0) which is 
the default for VARDECIMAL
+          .put("UNSIGNEDLONG", MinorType.VARDECIMAL)
+          .put("UNSIGNEDINT", MinorType.BIGINT)
+          .put("UNSIGNEDSHORT", MinorType.UINT2)
+          .put("UNSIGNEDBYTE", MinorType.UINT1)
+          // daffodil integer, nonNegativeInteger, are modeled as DECIMAL(38, 
0) which is the default for VARDECIMAL
+          .put("INTEGER", MinorType.VARDECIMAL)
+          .put("NONNEGATIVEINTEGER", MinorType.VARDECIMAL)
+          // decimal has to be modeled as string since we really have no idea 
what to set the
+          // scale to.
+          .put("DECIMAL", MinorType.VARCHAR)
+          .put("BOOLEAN", MinorType.BIT)
+          .put("DATE", MinorType.DATE) // requires conversion
+          .put("DATETIME", MinorType.TIMESTAMP) // requires conversion
+          .put("DOUBLE", MinorType.FLOAT8)
+          //
+          // daffodil float type is mapped to double aka Float8 in drill 
because there
+          // seems to be bugs in FLOAT4. Float.MaxValue in a Float4 column 
displays as
+          // 3.4028234663852886E38 not 3.4028235E38.
+          //
+          // We don't really care about single float precision, so we just use 
double precision.
+          //
+          .put("FLOAT", MinorType.FLOAT8)
+          .put("HEXBINARY", MinorType.VARBINARY)
+          .put("STRING", MinorType.VARCHAR)
+          .put("TIME", MinorType.TIME) // requires conversion
+          .build();
+
+
+  @VisibleForTesting
+  public static TupleMetadata processSchema(URI dfdlSchemaURI, String 
rootName, String namespace)
+      throws IOException, DaffodilDataProcessorFactory.CompileFailure,
+      URISyntaxException, InvalidParserException {
+    DaffodilDataProcessorFactory dpf = new DaffodilDataProcessorFactory();
+    DataProcessor dp = dpf.getDataProcessor(dfdlSchemaURI, true, rootName, 
namespace);

Review Comment:
   Add 
   ```
   boolean validationMode = true // use Daffodil's limited validation always
   ```
   instead of just passing true. 
   
   Document in the README.md that limited validation is always performed. 
   TBD: Document how validation errors are handled - escalated to errors, or 
issued as warnings? Is an option needed here?



##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/schema/DaffodilDataProcessorFactory.java:
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.daffodil.schema;
+
+import org.apache.daffodil.japi.Compiler;
+import org.apache.daffodil.japi.Daffodil;
+import org.apache.daffodil.japi.DataProcessor;
+import org.apache.daffodil.japi.Diagnostic;
+import org.apache.daffodil.japi.InvalidParserException;
+import org.apache.daffodil.japi.InvalidUsageException;
+import org.apache.daffodil.japi.ProcessorFactory;
+import org.apache.daffodil.japi.ValidationMode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.channels.Channels;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * Compiles a DFDL schema (mostly for tests) or loads a pre-compiled DFDL 
schema so that one can
+ * obtain a DataProcessor for use with DaffodilMessageParser.
+ * <p/>
+ * TODO: Needs to use a cache to avoid reloading/recompiling every time.
+ */
+public class DaffodilDataProcessorFactory {
+  // Default constructor is used.
+
+  private static final Logger logger = 
LoggerFactory.getLogger(DaffodilDataProcessorFactory.class);
+
+  private DataProcessor dp;
+
+  /**
+   * Gets a Daffodil DataProcessor given the necessary arguments to compile or 
reload it.
+   *
+   * @param schemaFileURI
+   *     pre-compiled dfdl schema (.bin extension) or DFDL schema source (.xsd 
extension)
+   * @param validationMode
+   *     Use true to request Daffodil built-in 'limited' validation. Use false 
for no validation.
+   * @param rootName
+   *     Local name of root element of the message. Can be null to use the 
first element declaration
+   *     of the primary schema file. Ignored if reloading a pre-compiled 
schema.
+   * @param rootNS
+   *     Namespace URI as a string. Can be null to use the target namespace of 
the primary schema
+   *     file or if it is unambiguous what element is the rootName. Ignored if 
reloading a
+   *     pre-compiled schema.
+   * @return the DataProcessor
+   * @throws CompileFailure
+   *     - if schema compilation fails
+   */
+  public DataProcessor getDataProcessor(URI schemaFileURI, boolean 
validationMode, String rootName,
+      String rootNS)
+      throws CompileFailure {
+
+    DaffodilDataProcessorFactory dmp = new DaffodilDataProcessorFactory();
+    boolean isPrecompiled = schemaFileURI.toString().endsWith(".bin");
+    if (isPrecompiled) {
+      if (Objects.nonNull(rootName) && !rootName.isEmpty()) {
+        // A usage error. You shouldn't supply the name and optionally 
namespace if loading
+        // precompiled schema because those are built into it. Should be null 
or "".
+        logger.warn("Root element name '{}' is ignored when used with 
precompiled DFDL schema.",
+            rootName);
+      }
+      try {
+        dmp.loadSchema(schemaFileURI);
+      } catch (IOException | InvalidParserException e) {
+        throw new CompileFailure(e);

Review Comment:
   Error architecture? 
   
   This loadSchema call needs to happen on every node, and so has the potential 
(if the loaded binary schema file is no good or mismatches the Daffodil library 
version) to fail. Is throwing this exception the right thing here or are other 
steps preferred/necessary?



##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/DaffodilBatchReader.java:
##########
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.drill.exec.store.daffodil;
+
+import org.apache.daffodil.japi.DataProcessor;
+import org.apache.drill.common.AutoCloseables;
+import org.apache.drill.common.exceptions.CustomErrorContext;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.exec.physical.impl.scan.v3.ManagedReader;
+import org.apache.drill.exec.physical.impl.scan.v3.file.FileDescrip;
+import org.apache.drill.exec.physical.impl.scan.v3.file.FileSchemaNegotiator;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import 
org.apache.drill.exec.store.daffodil.schema.DaffodilDataProcessorFactory;
+import org.apache.drill.exec.store.dfs.DrillFileSystem;
+import org.apache.drill.exec.store.dfs.easy.EasySubScan;
+import org.apache.hadoop.fs.Path;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.util.Objects;
+
+import static 
org.apache.drill.exec.store.daffodil.schema.DaffodilDataProcessorFactory.*;
+import static 
org.apache.drill.exec.store.daffodil.schema.DrillDaffodilSchemaUtils.daffodilDataProcessorToDrillSchema;
+
+public class DaffodilBatchReader implements ManagedReader {
+
+  private static final Logger logger = 
LoggerFactory.getLogger(DaffodilBatchReader.class);
+  private final RowSetLoader rowSetLoader;
+  private final CustomErrorContext errorContext;
+  private final DaffodilMessageParser dafParser;
+  private final InputStream dataInputStream;
+
+  public DaffodilBatchReader(DaffodilReaderConfig readerConfig, EasySubScan 
scan,
+      FileSchemaNegotiator negotiator) {
+
+    errorContext = negotiator.parentErrorContext();
+    DaffodilFormatConfig dafConfig = readerConfig.plugin.getConfig();
+
+    String schemaURIString = dafConfig.getSchemaURI(); // 
"schema/complexArray1.dfdl.xsd";
+    String rootName = dafConfig.getRootName();
+    String rootNamespace = dafConfig.getRootNamespace();
+    boolean validationMode = dafConfig.getValidationMode();
+
+    URI dfdlSchemaURI;
+    try {
+      dfdlSchemaURI = new URI(schemaURIString);
+    } catch (URISyntaxException e) {
+      throw UserException.validationError(e).build(logger);
+    }
+
+    FileDescrip file = negotiator.file();
+    DrillFileSystem fs = file.fileSystem();
+    URI fsSchemaURI = fs.getUri().resolve(dfdlSchemaURI);
+
+    DaffodilDataProcessorFactory dpf = new DaffodilDataProcessorFactory();
+    DataProcessor dp;
+    try {
+      dp = dpf.getDataProcessor(fsSchemaURI, validationMode, rootName, 
rootNamespace);
+    } catch (CompileFailure e) {
+      throw UserException.dataReadError(e)
+          .message(String.format("Failed to get Daffodil DFDL processor for: 
%s", fsSchemaURI))
+          .addContext(errorContext).addContext(e.getMessage()).build(logger);
+    }
+    // Create the corresponding Drill schema.
+    // Note: this could be a very large schema. Think of a large complex RDBMS 
schema,
+    // all of it, hundreds of tables, but all part of the same metadata tree.
+    TupleMetadata drillSchema = daffodilDataProcessorToDrillSchema(dp);
+    // Inform Drill about the schema
+    negotiator.tableSchema(drillSchema, true);
+
+    //
+    // DATA TIME: Next we construct the runtime objects, and open files.
+    //
+    // We get the DaffodilMessageParser, which is a stateful driver for 
daffodil that
+    // actually does the parsing.
+    rowSetLoader = negotiator.build().writer();
+
+    // We construct the Daffodil InfosetOutputter which the daffodil parser 
uses to
+    // convert infoset event calls to fill in a Drill row via a rowSetLoader.
+    DaffodilDrillInfosetOutputter outputter = new 
DaffodilDrillInfosetOutputter(rowSetLoader);
+
+    // Now we can set up the dafParser with the outputter it will drive with
+    // the parser-produced infoset.
+    dafParser = new DaffodilMessageParser(dp); // needs further initialization 
after this.
+    dafParser.setInfosetOutputter(outputter);
+
+    Path dataPath = file.split().getPath();
+    // Lastly, we open the data stream
+    try {
+      dataInputStream = fs.openPossiblyCompressedStream(dataPath);
+    } catch (IOException e) {
+      throw UserException.dataReadError(e)
+          .message(String.format("Failed to open input file: %s", 
dataPath.toString()))
+          .addContext(errorContext).addContext(e.getMessage()).build(logger);
+    }
+    // And lastly,... tell daffodil the input data stream.
+    dafParser.setInputStream(dataInputStream);
+  }
+
+  /**
+   * This is the core of actual processing - data movement from Daffodil to 
Drill.
+   * <p>
+   * If there is space in the batch, and there is data available to parse then 
this calls the
+   * daffodil parser, which parses data, delivering it to the rowWriter by way 
of the infoset
+   * outputter.
+   * <p>
+   * Repeats until the rowWriter is full (a batch is full), or there is no 
more data, or a parse
+   * error ends execution with a throw.
+   * <p>
+   * Validation errors and other warnings are not errors and are logged but do 
not cause parsing to
+   * fail/throw.
+   *
+   * @return true if there are rows retrieved, false if no rows were 
retrieved, which means no more
+   *     will ever be retrieved (end of data).
+   * @throws RuntimeException
+   *     on parse errors.
+   */
+  @Override
+  public boolean next() {
+    // Check assumed invariants
+    // We don't know if there is data or not. This could be called on an empty 
data file.
+    // We DO know that this won't be called if there is no space in the batch 
for even 1
+    // row.
+    if (dafParser.isEOF()) {
+      return false; // return without even checking for more rows or trying to 
parse.
+    }
+    while (rowSetLoader.start() && !dafParser.isEOF()) { // we never zero-trip 
this loop.
+      // the predicate is always true once.
+      dafParser.parse();
+      if (dafParser.isProcessingError()) {
+        assert (Objects.nonNull(dafParser.getDiagnostics()));
+        throw 
UserException.dataReadError().message(dafParser.getDiagnosticsAsString())
+            .addContext(errorContext).build(logger);
+      }
+      if (dafParser.isValidationError()) {
+        logger.warn(dafParser.getDiagnosticsAsString());

Review Comment:
   Do we need an option here to convert validation errors to fatal?
   
   Will logger.warn be seen by a query user, or is that just for someone 
dealing with the logs?
   
   Validation errors either should be escalated to fatal, OR they should be 
visible in the query output display to a user somehow. 
   
   Either way, users will need a mechanism to suppress validation errors that 
prove to be unavoidable since they could be common place. Nodody wants 
thousands of warnings about something they can't avoid that doesn't stop 
parsing and querying the data. 



##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/schema/DaffodilDataProcessorFactory.java:
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.daffodil.schema;
+
+import org.apache.daffodil.japi.Compiler;
+import org.apache.daffodil.japi.Daffodil;
+import org.apache.daffodil.japi.DataProcessor;
+import org.apache.daffodil.japi.Diagnostic;
+import org.apache.daffodil.japi.InvalidParserException;
+import org.apache.daffodil.japi.InvalidUsageException;
+import org.apache.daffodil.japi.ProcessorFactory;
+import org.apache.daffodil.japi.ValidationMode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.channels.Channels;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * Compiles a DFDL schema (mostly for tests) or loads a pre-compiled DFDL 
schema so that one can
+ * obtain a DataProcessor for use with DaffodilMessageParser.
+ * <p/>
+ * TODO: Needs to use a cache to avoid reloading/recompiling every time.
+ */
+public class DaffodilDataProcessorFactory {
+  // Default constructor is used.
+
+  private static final Logger logger = 
LoggerFactory.getLogger(DaffodilDataProcessorFactory.class);
+
+  private DataProcessor dp;
+
+  /**
+   * Gets a Daffodil DataProcessor given the necessary arguments to compile or 
reload it.
+   *
+   * @param schemaFileURI
+   *     pre-compiled dfdl schema (.bin extension) or DFDL schema source (.xsd 
extension)
+   * @param validationMode
+   *     Use true to request Daffodil built-in 'limited' validation. Use false 
for no validation.
+   * @param rootName
+   *     Local name of root element of the message. Can be null to use the 
first element declaration
+   *     of the primary schema file. Ignored if reloading a pre-compiled 
schema.
+   * @param rootNS
+   *     Namespace URI as a string. Can be null to use the target namespace of 
the primary schema
+   *     file or if it is unambiguous what element is the rootName. Ignored if 
reloading a
+   *     pre-compiled schema.
+   * @return the DataProcessor
+   * @throws CompileFailure
+   *     - if schema compilation fails
+   */
+  public DataProcessor getDataProcessor(URI schemaFileURI, boolean 
validationMode, String rootName,
+      String rootNS)
+      throws CompileFailure {
+
+    DaffodilDataProcessorFactory dmp = new DaffodilDataProcessorFactory();
+    boolean isPrecompiled = schemaFileURI.toString().endsWith(".bin");
+    if (isPrecompiled) {
+      if (Objects.nonNull(rootName) && !rootName.isEmpty()) {
+        // A usage error. You shouldn't supply the name and optionally 
namespace if loading
+        // precompiled schema because those are built into it. Should be null 
or "".
+        logger.warn("Root element name '{}' is ignored when used with 
precompiled DFDL schema.",
+            rootName);
+      }
+      try {
+        dmp.loadSchema(schemaFileURI);
+      } catch (IOException | InvalidParserException e) {
+        throw new CompileFailure(e);
+      }
+      dmp.setupDP(validationMode, null);
+    } else {
+      List<Diagnostic> pfDiags;
+      try {
+        pfDiags = dmp.compileSchema(schemaFileURI, rootName, rootNS);
+      } catch (URISyntaxException | IOException e) {
+        throw new CompileFailure(e);
+      }
+      dmp.setupDP(validationMode, pfDiags);
+    }
+    return dmp.dp;
+  }
+
+  private void loadSchema(URI schemaFileURI) throws IOException, 
InvalidParserException {
+    Compiler c = Daffodil.compiler();
+    dp = c.reload(Channels.newChannel(schemaFileURI.toURL().openStream()));

Review Comment:
   @cgivre This reload call is the one that has to happen on every drill node. 
   It needs only to happen once for that schema for the life of the JVM. The 
"dp" object created here can be reused every time that schema is needed to 
parse more data. The dp (DataProcessor) is a read only (thread safe) data 
structure. 
   
   As you see, this can throw exceptions, so the question of how those 
situations should be handled arises. 
   Even if drill perfectly makes the file available to every node for this, 
that would rule out the IOException due to file not found or access rights, but 
a user can create a compiled DFDL schema binary file using the wrong version of 
the Daffodil schema compiler which is a mismatch for the runtime; hence, it is 
possible for the InvalidParserException to be thrown. 



##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/schema/DaffodilDataProcessorFactory.java:
##########
@@ -0,0 +1,162 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.drill.exec.store.daffodil.schema;
+
+import org.apache.daffodil.japi.Compiler;
+import org.apache.daffodil.japi.Daffodil;
+import org.apache.daffodil.japi.DataProcessor;
+import org.apache.daffodil.japi.Diagnostic;
+import org.apache.daffodil.japi.InvalidParserException;
+import org.apache.daffodil.japi.InvalidUsageException;
+import org.apache.daffodil.japi.ProcessorFactory;
+import org.apache.daffodil.japi.ValidationMode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.net.URI;
+import java.net.URISyntaxException;
+import java.nio.channels.Channels;
+import java.util.List;
+import java.util.Objects;
+
+/**
+ * Compiles a DFDL schema (mostly for tests) or loads a pre-compiled DFDL 
schema so that one can
+ * obtain a DataProcessor for use with DaffodilMessageParser.
+ * <p/>
+ * TODO: Needs to use a cache to avoid reloading/recompiling every time.
+ */
+public class DaffodilDataProcessorFactory {
+  // Default constructor is used.
+
+  private static final Logger logger = 
LoggerFactory.getLogger(DaffodilDataProcessorFactory.class);
+
+  private DataProcessor dp;
+
+  /**
+   * Gets a Daffodil DataProcessor given the necessary arguments to compile or 
reload it.
+   *
+   * @param schemaFileURI
+   *     pre-compiled dfdl schema (.bin extension) or DFDL schema source (.xsd 
extension)
+   * @param validationMode
+   *     Use true to request Daffodil built-in 'limited' validation. Use false 
for no validation.
+   * @param rootName
+   *     Local name of root element of the message. Can be null to use the 
first element declaration
+   *     of the primary schema file. Ignored if reloading a pre-compiled 
schema.
+   * @param rootNS
+   *     Namespace URI as a string. Can be null to use the target namespace of 
the primary schema
+   *     file or if it is unambiguous what element is the rootName. Ignored if 
reloading a
+   *     pre-compiled schema.
+   * @return the DataProcessor
+   * @throws CompileFailure
+   *     - if schema compilation fails
+   */
+  public DataProcessor getDataProcessor(URI schemaFileURI, boolean 
validationMode, String rootName,
+      String rootNS)
+      throws CompileFailure {
+
+    DaffodilDataProcessorFactory dmp = new DaffodilDataProcessorFactory();
+    boolean isPrecompiled = schemaFileURI.toString().endsWith(".bin");
+    if (isPrecompiled) {
+      if (Objects.nonNull(rootName) && !rootName.isEmpty()) {
+        // A usage error. You shouldn't supply the name and optionally 
namespace if loading
+        // precompiled schema because those are built into it. Should be null 
or "".
+        logger.warn("Root element name '{}' is ignored when used with 
precompiled DFDL schema.",
+            rootName);
+      }
+      try {
+        dmp.loadSchema(schemaFileURI);
+      } catch (IOException | InvalidParserException e) {
+        throw new CompileFailure(e);
+      }
+      dmp.setupDP(validationMode, null);
+    } else {
+      List<Diagnostic> pfDiags;
+      try {
+        pfDiags = dmp.compileSchema(schemaFileURI, rootName, rootNS);
+      } catch (URISyntaxException | IOException e) {
+        throw new CompileFailure(e);
+      }
+      dmp.setupDP(validationMode, pfDiags);
+    }
+    return dmp.dp;
+  }
+
+  private void loadSchema(URI schemaFileURI) throws IOException, 
InvalidParserException {
+    Compiler c = Daffodil.compiler();
+    dp = c.reload(Channels.newChannel(schemaFileURI.toURL().openStream()));
+  }
+
+  private List<Diagnostic> compileSchema(URI schemaFileURI, String rootName, 
String rootNS)
+      throws URISyntaxException, IOException, CompileFailure {
+    Compiler c = Daffodil.compiler();
+    ProcessorFactory pf = c.compileSource(schemaFileURI, rootName, rootNS);
+    List<Diagnostic> pfDiags = pf.getDiagnostics();
+    if (pf.isError()) {
+      pfDiags.forEach(diag -> logger.error(diag.getSomeMessage()));
+      throw new CompileFailure(pfDiags);
+    }
+    dp = pf.onPath("/");
+    return pfDiags; // must be just warnings. If it was errors we would have 
thrown.
+  }
+
+  /**
+   * Common setup steps used whether or not we reloaded or compiled a DFDL 
schema.
+   */
+  private void setupDP(boolean validationMode, List<Diagnostic> pfDiags) 
throws CompileFailure {
+    Objects.requireNonNull(dp); // true because failure to produce a dp throws 
CompileFailure.
+    if (validationMode) {
+      try {
+        dp = dp.withValidationMode(ValidationMode.Limited);

Review Comment:
   Add comments about why this is ValidationMode.Limited only. (We don't have 
the schema text, and aren't creating any XML so using the Full Xerces-based 
validator is not possible.)





> Add Daffodil Format Plugin
> --------------------------
>
>                 Key: DRILL-8474
>                 URL: https://issues.apache.org/jira/browse/DRILL-8474
>             Project: Apache Drill
>          Issue Type: New Feature
>    Affects Versions: 1.21.1
>            Reporter: Charles Givre
>            Priority: Major
>             Fix For: 1.22.0
>
>




--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to