[jira] [Commented] (DRILL-8188) Convert HDF5 format to EVF2

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

jnturton merged PR #2515:
URL: https://github.com/apache/drill/pull/2515




> Convert HDF5 format to EVF2
> ---
>
> Key: DRILL-8188
> URL: https://issues.apache.org/jira/browse/DRILL-8188
> Project: Apache Drill
>  Issue Type: Improvement
>Affects Versions: 1.20.0
>Reporter: Cong Luo
>Assignee: Cong Luo
>Priority: Major
>
> Use EVF V2 instead of old V1.
> Also, fixed a few bugs in V2 framework.



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


[jira] [Commented] (DRILL-8188) Convert HDF5 format to EVF2

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

jnturton commented on code in PR #2515:
URL: https://github.com/apache/drill/pull/2515#discussion_r1446231938


##
contrib/format-hdf5/src/main/java/org/apache/drill/exec/store/hdf5/HDF5BatchReader.java:
##
@@ -171,107 +168,109 @@ public HDF5ReaderConfig(HDF5FormatPlugin plugin, 
HDF5FormatConfig formatConfig)
 }
   }
 
-  public HDF5BatchReader(HDF5ReaderConfig readerConfig, int maxRecords) {
-this.readerConfig = readerConfig;
-this.maxRecords = maxRecords;
+  public HDF5BatchReader(HDF5ReaderConfig config, EasySubScan scan, 
FileSchemaNegotiator negotiator) {
+errorContext = negotiator.parentErrorContext();
+file = negotiator.file();
+readerConfig = config;
 dataWriters = new ArrayList<>();
-this.showMetadataPreview = readerConfig.formatConfig.showPreview();
-  }
+showMetadataPreview = readerConfig.formatConfig.showPreview();
 
-  @Override
-  public boolean open(FileSchemaNegotiator negotiator) {
-split = negotiator.split();
-errorContext = negotiator.parentErrorContext();
 // Since the HDF file reader uses a stream to actually read the file, the 
file name from the
 // module is incorrect.
-fileName = split.getPath().getName();
-try {
-  openFile(negotiator);
-} catch (IOException e) {
-  throw UserException
-.dataReadError(e)
-.addContext("Failed to close input file: %s", split.getPath())
-.addContext(errorContext)
-.build(logger);
-}
+fileName = file.split().getPath().getName();
 
-ResultSetLoader loader;
-if (readerConfig.defaultPath == null) {
-  // Get file metadata
-  List metadata = getFileMetadata(hdfFile, new 
ArrayList<>());
-  metadataIterator = metadata.iterator();
-
-  // Schema for Metadata query
-  SchemaBuilder builder = new SchemaBuilder()
-.addNullable(PATH_COLUMN_NAME, MinorType.VARCHAR)
-.addNullable(DATA_TYPE_COLUMN_NAME, MinorType.VARCHAR)
-.addNullable(FILE_NAME_COLUMN_NAME, MinorType.VARCHAR)
-.addNullable(DATA_SIZE_COLUMN_NAME, MinorType.BIGINT)
-.addNullable(IS_LINK_COLUMN_NAME, MinorType.BIT)
-.addNullable(ELEMENT_COUNT_NAME, MinorType.BIGINT)
-.addNullable(DATASET_DATA_TYPE_NAME, MinorType.VARCHAR)
-.addNullable(DIMENSIONS_FIELD_NAME, MinorType.VARCHAR);
-
-  negotiator.tableSchema(builder.buildSchema(), false);
-
-  loader = negotiator.build();
-  dimensions = new int[0];
-  rowWriter = loader.writer();
-
-} else {
-  // This is the case when the default path is specified. Since the user 
is explicitly asking for a dataset
-  // Drill can obtain the schema by getting the datatypes below and 
ultimately mapping that schema to columns
-  Dataset dataSet = hdfFile.getDatasetByPath(readerConfig.defaultPath);
-  dimensions = dataSet.getDimensions();
-
-  loader = negotiator.build();
-  rowWriter = loader.writer();
-  writerSpec = new WriterSpec(rowWriter, negotiator.providedSchema(),
-  negotiator.parentErrorContext());
-  if (dimensions.length <= 1) {
-buildSchemaFor1DimensionalDataset(dataSet);
-  } else if (dimensions.length == 2) {
-buildSchemaFor2DimensionalDataset(dataSet);
-  } else {
-// Case for datasets of greater than 2D
-// These are automatically flattened
-buildSchemaFor2DimensionalDataset(dataSet);
+{ // Opens an HDF5 file

Review Comment:
   I guess some of these could become private methods but it's a minor point 
for me.



##
contrib/format-hdf5/src/main/java/org/apache/drill/exec/store/hdf5/HDF5BatchReader.java:
##
@@ -171,107 +164,104 @@ public HDF5ReaderConfig(HDF5FormatPlugin plugin, 
HDF5FormatConfig formatConfig)
 }
   }
 
-  public HDF5BatchReader(HDF5ReaderConfig readerConfig, int maxRecords) {
-this.readerConfig = readerConfig;
-this.maxRecords = maxRecords;
+  public HDF5BatchReader(HDF5ReaderConfig config, EasySubScan scan, 
FileSchemaNegotiator negotiator) {
+errorContext = negotiator.parentErrorContext();
+file = negotiator.file();
+readerConfig = config;
 dataWriters = new ArrayList<>();
-this.showMetadataPreview = readerConfig.formatConfig.showPreview();
-  }
+showMetadataPreview = readerConfig.formatConfig.showPreview();
 
-  @Override
-  public boolean open(FileSchemaNegotiator negotiator) {
-split = negotiator.split();
-errorContext = negotiator.parentErrorContext();
 // Since the HDF file reader uses a stream to actually read the file, the 
file name from the
 // module is incorrect.
-fileName = split.getPath().getName();
-try {
-  openFile(negotiator);
-} catch (IOException e) {
-  throw 

[jira] [Commented] (DRILL-8475) The binary version License and NOTICE do not comply with the corresponding terms.

2024-01-14 Thread Calvin Kirs (Jira)


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

Calvin Kirs commented on DRILL-8475:


[~dzamo] Hi James,I think ok
 
 

> The binary version License and NOTICE do not comply with the corresponding 
> terms.
> -
>
> Key: DRILL-8475
> URL: https://issues.apache.org/jira/browse/DRILL-8475
> Project: Apache Drill
>  Issue Type: Bug
>Affects Versions: 1.21.1
>Reporter: Calvin Kirs
>Assignee: James Turton
>Priority: Blocker
> Fix For: 1.21.2
>
> Attachments: dependencies.txt, drill-dep-list.txt
>
>
> I checked the latest released version, and it does not follow the 
> corresponding rules[1]. This is very important and I hope it will be taken 
> seriously by the PMC team. I'd be happy to do it if needed.
> [1] [https://infra.apache.org/licensing-howto.html#binary]
>  
>  



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


[jira] [Commented] (DRILL-8475) The binary version License and NOTICE do not comply with the corresponding terms.

2024-01-14 Thread Calvin Kirs (Jira)


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

Calvin Kirs commented on DRILL-8475:


I executed mvn clean install -DskipTests -Phadoop-2, then checked the 
dependency file in our jar package in the jars directory, and then filtered it 
to get the following results.

> The binary version License and NOTICE do not comply with the corresponding 
> terms.
> -
>
> Key: DRILL-8475
> URL: https://issues.apache.org/jira/browse/DRILL-8475
> Project: Apache Drill
>  Issue Type: Bug
>Affects Versions: 1.21.1
>Reporter: Calvin Kirs
>Assignee: James Turton
>Priority: Blocker
> Fix For: 1.21.2
>
> Attachments: dependencies.txt, drill-dep-list.txt
>
>
> I checked the latest released version, and it does not follow the 
> corresponding rules[1]. This is very important and I hope it will be taken 
> seriously by the PMC team. I'd be happy to do it if needed.
> [1] [https://infra.apache.org/licensing-howto.html#binary]
>  
>  



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


[jira] [Updated] (DRILL-8475) The binary version License and NOTICE do not comply with the corresponding terms.

2024-01-14 Thread Calvin Kirs (Jira)


 [ 
https://issues.apache.org/jira/browse/DRILL-8475?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Calvin Kirs updated DRILL-8475:
---
Attachment: dependencies.txt

> The binary version License and NOTICE do not comply with the corresponding 
> terms.
> -
>
> Key: DRILL-8475
> URL: https://issues.apache.org/jira/browse/DRILL-8475
> Project: Apache Drill
>  Issue Type: Bug
>Affects Versions: 1.21.1
>Reporter: Calvin Kirs
>Assignee: James Turton
>Priority: Blocker
> Fix For: 1.21.2
>
> Attachments: dependencies.txt, drill-dep-list.txt
>
>
> I checked the latest released version, and it does not follow the 
> corresponding rules[1]. This is very important and I hope it will be taken 
> seriously by the PMC team. I'd be happy to do it if needed.
> [1] [https://infra.apache.org/licensing-howto.html#binary]
>  
>  



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


[jira] [Commented] (DRILL-8474) Add Daffodil Format Plugin

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

cgivre commented on PR #2836:
URL: https://github.com/apache/drill/pull/2836#issuecomment-1890990577

   > > @mbeckerle With respect to style, I tried to reply to that comment, but 
the thread won't let me. In any event, Drill classes will typically start with 
the constructor, then have whatever methods are appropriate for the class. The 
logger creation usually happens before the constructor. I think all of your 
other classes followed this format, so the one or two that didn't kind of 
jumped out at me.
   > 
   > @cgivre I believe the style issues are all fixed. The build did not get 
any codestyle issues.
   
   The issue I was referring to was more around the organization of a few 
classes.  Usually we'll have the constructor (if present) at the top followed 
by any class methods.  I think there was a class or two where the constructor 
was at the bottom or something like that.  In any event, consider the issue 
resolved.




> 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)


[jira] [Commented] (DRILL-8474) Add Daffodil Format Plugin

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

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


##
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 

[jira] [Commented] (DRILL-8474) Add Daffodil Format Plugin

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

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


##
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:
   This is likely music to @paul-rogers's ears.





> 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)


[jira] [Commented] (DRILL-8474) Add Daffodil Format Plugin

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

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


##
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.
+ * 
+ * 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 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:
   This definitely seems like an area where there is potential for a lot of 
different things to go wrong.  My view is we should just do our best to provide 
clear error 

[jira] [Commented] (DRILL-8474) Add Daffodil Format Plugin

2024-01-14 Thread ASF GitHub Bot (Jira)


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

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

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


##
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.
+ * 
+ * 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:
   My thought here would be to fail as quickly as possible.  If the DFDL schema 
can't be read, I'm assuming that we cannot proceed, so throwing an exception 
would be the right thing to do IMHO.  With that said, we should make sure we 
provide a good error message that would explain what went wrong. 
   One of the issues we worked on for a while with Drill was that it would fail 
and you'd get a stack trace w/o a clear idea of what the actual issue is and 
how to rectify it. 
   





> Add Daffodil Format Plugin
> --
>
> Key: DRILL-8474
> URL: https://issues.apache.org/jira/browse/DRILL-8474
> Project: Apache Drill
>  Issue Type: New Feature
>