[
https://issues.apache.org/jira/browse/DRILL-8474?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18058460#comment-18058460
]
ASF GitHub Bot commented on DRILL-8474:
---------------------------------------
stevedlawrence commented on code in PR #2989:
URL: https://github.com/apache/drill/pull/2989#discussion_r2804134130
##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/schema/DaffodilDataProcessorFactory.java:
##########
@@ -0,0 +1,165 @@
+/*
+ * 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:
Just to be clear, since I don't think Daffodil documents this very well, the
way save/reload works in Daffodil is `save` uses Java serialization to
serialize a DataProcessor, and `reload` just deserializes it. In all uses that
I'm aware of, these .bin files are trusted and so are safe to
reload/deserialize. But I know there are a number of security concerns with
deserializing untrusted data. If the Drill security model doesn't necessarily
trust users with files provided to `CREATE DAFFODIL SCHEMA` or `SELECT * from
... type=daffodil`, then it might be worth considering if reload is something
you want to disallow. It's a potential loss since saved parsers are easier to
distribute and can avoid long schema compilation times, but it does come with
potential risk.
##########
contrib/format-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/DaffodilBatchReader.java:
##########
@@ -0,0 +1,197 @@
+/*
+ * 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.CompileFailure;
+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 schemaFile = dafConfig.getSchemaFile();
+ String schemaURIString = dafConfig.getSchemaURI();
+ String rootName = dafConfig.getRootName();
+ String rootNamespace = dafConfig.getRootNamespace();
+ boolean validationMode = dafConfig.getValidationMode();
+
+ // Determine the schema URI:
+ // - If schemaFile is provided, it takes precedence and is looked up in
the registry area
+ // - Otherwise, use schemaURI (full path)
+ URI dfdlSchemaURI;
+ try {
+ if (schemaFile != null && !schemaFile.isEmpty()) {
+ // schemaFile takes precedence - construct path from registry area
+ Path registryArea = readerConfig.plugin.getContext()
+ .getRemoteDaffodilSchemaRegistry().getRegistryArea();
+ Path schemaPath = new Path(registryArea, schemaFile);
+ dfdlSchemaURI = schemaPath.toUri();
+ } else if (schemaURIString != null && !schemaURIString.isEmpty()) {
+ // Use the provided schemaURI
+ dfdlSchemaURI = new URI(schemaURIString);
+ } else {
+ // Neither provided - will result in empty URI
+ dfdlSchemaURI = new URI("");
+ }
+ } 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.
Review Comment:
One thing to consider is that it is possible for Daffodil to consume zero
bits of data and it be considered a success. Schemas that allow that are kindof
a pathological case and don't really exist in the real world, but might be
something to consider since it is technically possible.
The way to detect this is to access `ParseResult.location().bitPos1b()` at
the end of a parse. If that bit position is the same as before the parse then
it means the parse successfully consumed zero bits, and it will continue to do
so in an infinite loop if you keep attempting to parse. For example, this
usually looks something like this:
```java
int lastBitPosition = 1;
while (...)
ParseResult pr = dp.parse();
... // check isError
int currBitPosition = pr.location().bitPos1b();
if (currBitPosition == lastBitPosition)) {
// consumed no data, exit parsing to avoid infinite loop
break;
}
lastBitPosition = currBitPosition;
}
```
Note that even if Daffodil consumes zero bits, it is still possible to
create infoset events, so you really do need to check the bit position instead
checking if infoset events were created.
> 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
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
