DRILL-8474: Adding Daffodil to Drill as a contrib. #2909

mbeckerle · 2024-04-27T20:40:20Z

DRILL-8474: Adding Daffodil to Drill as a contrib.

This PR replaces: #2836 which is closed. That was to retain history/comments while squashing numerous debug-related commits together into this PR.

Description

Requires Daffodil version 3.7.0 or higher.

New format-daffodil module created

Still uses absolute paths for the schemaFileURI.
(which is cheating. Wouldn't work in a true distributed drill environment.)

We have yet to work out how to enable Drill to provide access for DFDL schemas in XML form with include/import to be resolved.

The input data stream is, however, being accessed in the proper Drill manner. Gunzip happened automatically. Nice.

Note: Fix boxed Boolean vs. boolean problem. Don't use boxed primitives in Format config objects.

Test show this works for data as complex as having nested repeating sub-records.

These DFDL types are supported:

int
long
short
byte
boolean
double
float (does not work. Bug DAFFODIL-2367)
hexBinary
string

Documentation

TBD: feature is incomplete still. It will require substantial documentation for users.

Testing

See tests under src/test in the new daffodil contrib module.

Requires Daffodil version 3.7.0 or higher. New format-daffodil module created Still uses absolute paths for the schemaFileURI. (which is cheating. Wouldn't work in a true distributed drill environment.) We have yet to work out how to enable Drill to provide access for DFDL schemas in XML form with include/import to be resolved. The input data stream is, however, being accessed in the proper Drill manner. Gunzip happened automatically. Nice. Note: Fix boxed Boolean vs. boolean problem. Don't use boxed primitives in Format config objects. Test show this works for data as complex as having nested repeating sub-records. These DFDL types are supported: - int - long - short - byte - boolean - double - float (does not work. Bug DAFFODIL-2367) - hexBinary - string apache#2835

mbeckerle · 2024-04-27T20:42:02Z

contrib/format-daffodil/pom.xml

+  <build>
+    <plugins>
+      <plugin>
+        <artifactId>maven-resources-plugin</artifactId>
+        <executions>
+          <execution>
+            <id>copy-java-sources</id>
+            <phase>process-sources</phase>
+            <goals>
+              <goal>copy-resources</goal>
+            </goals>
+            <configuration>
+              <outputDirectory>${basedir}/target/classes/org/apache/drill/exec/store/daffodil
+              </outputDirectory>
+              <resources>
+                <resource>
+                  <directory>src/main/java/org/apache/drill/exec/store/daffodil</directory>
+                  <filtering>true</filtering>
+                </resource>
+              </resources>
+            </configuration>
+          </execution>
+        </executions>
+      </plugin>
+    </plugins>
+  </build>


What does this section do? I noticed that the msaccess contrib doesn't have this section in its pom.xml.

mbeckerle · 2024-04-27T20:51:56Z

This fails its tests due to a maven checkstyle failure. It's complaining about Drill:Exec:Vectors, which my code has no changes to.

Can someone advise on what is wrong here?

shfshihuafeng · 2024-04-28T12:51:05Z

This fails its tests due to a maven checkstyle failure. It's complaining about Drill:Exec:Vectors, which my code has no changes to.

Can someone advise on what is wrong here?

/home/runner/work/drill/drill/exec/vector/src/main/java/org/apache/drill/exec/record/metadata/MapBuilder.java:201:5:
you should use '{} for if' construct

 if (Objects.isNull(parent)) {
    throw new IllegalStateException("Call to resume() on MapBuilder with no parent.");
}

cgivre · 2024-04-28T15:28:41Z

exec/vector/src/main/java/org/apache/drill/exec/record/metadata/MapBuilder.java

+   */
+  @Override
+  public void resume() {
+    if (Objects.isNull(parent))


@mbeckerle I don't know why the checkstyle is telling you the wrong file, but here, you'll need braces as well as at line 203.

ie:

if (parent instanceof MapBuilder) { resumeMap(); }

@mbeckerle Confirmed. I successfully built your branch by adding the aforementioned braces. I'll save you some additional trouble. There's another check style violation in DaffodilBatchReader. Drill doesn't like star imports for some reason.

What is the maven command line to just make it run this checkstyle?

I just built Drill using the following command:

mvn clean install -DskipTests

When I did that, I was getting the same error as on GitHub. After adding the braces as described above, it built without issues.
With that said, I think you can do just run the check style with:

mvn checkstyle:checkstyle

mbeckerle · 2024-04-29T02:11:08Z

Tests are now failing due to these two things in TestDaffodilReader.scala

  String schemaURIRoot = "file:///opt/drill/contrib/format-daffodil/src/test/resources/";

That's an absolute URI that is used to obtain access to the schema files in this statement:

  private String selectRow(String schema, String file) {
    return "SELECT * FROM table(dfs.`data/" + file + "` " + " (type => 'daffodil'," + " " +
        "validationMode => 'true', " + " schemaURI => '" + schemaURIRoot + "schema/" + schema +
        ".dfdl.xsd'," + " rootName => 'row'," + " rootNamespace => null " + "))";
  }

This is assembling a select statement, and puts this absolute schemaURI into the schemaURI part of the select.

What should I be doing to arrange for these schema URIs to be found.

The schemas are a large complex set of files, not just a single file. Many files must be found relative to the initial root schema file. (Hundreds of files potentially). As they include/import other schema files using relative paths.

cgivre · 2024-05-06T01:16:47Z

Hi Mike, Are you free at all this week? My apologies... We're in the middle of putting an offer on a house and my life is very hectic at the moment. Best, -- C

…

On Apr 28, 2024, at 10:11 PM, Mike Beckerle ***@***.***> wrote: Tests are now failing due to these two things in TestDaffodilReader.scala String schemaURIRoot = "file:///opt/drill/contrib/format-daffodil/src/test/resources/"; That's an absolute URI that is used to obtain access to the schema files in this statement: private String selectRow(String schema, String file) { return "SELECT * FROM table(dfs.`data/" + file + "` " + " (type => 'daffodil'," + " " + "validationMode => 'true', " + " schemaURI => '" + schemaURIRoot + "schema/" + schema + ".dfdl.xsd'," + " rootName => 'row'," + " rootNamespace => null " + "))"; } This is assembling a select statement, and puts this absolute schemaURI into the schemaURI part of the select. What should I be doing to arrange for these schema URIs to be found. The schemas are a large complex set of files, not just a single file. Many files must be found relative to the initial root schema file. (Hundreds of files potentially). As they include/import other schema files using relative paths. — Reply to this email directly, view it on GitHub <#2909 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABKB7PT327D7FTY34D7Z6ULY7WT5DAVCNFSM6AAAAABG4LUKI6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAOBRG44DCNJUGY>. You are receiving this because you commented.

mbeckerle · 2024-05-06T18:35:20Z

Hi Mike, Are you free at all this week? My apologies... We're in the middle of putting an offer on a house and my life is very hectic at the moment. Best, -- C

Lots of availability. I'll send you separate email.

mbeckerle · 2024-05-09T19:40:56Z

...ormat-daffodil/src/main/java/org/apache/drill/exec/store/daffodil/DaffodilMessageParser.java

+ */
+public class DaffodilMessageParser {
+
+  private static final Logger logger = LoggerFactory.getLogger(DaffodilMessageParser.class);


This logger needs to be Drill's logger for log messages originating in the drill bit.

mbeckerle · 2024-05-09T19:49:51Z

.../src/main/java/org/apache/drill/exec/store/daffodil/schema/DaffodilDataProcessorFactory.java

+            rootName);
+      }
+      try {
+        dmp.loadSchema(schemaFileURI);


This right here is depending on the schemaFileURI being class path relative so that the schema, and things referenced from within the schema, are found on the classpath. Daffodil makes extensive use of Java's Service Provider Interface (SPI) to dynamically load its own flavor of UDFs, custom charsets, and "layer" definitions, all of which are compiled java/scala code in jars and those jars can have other dependent jars, and so on.

So somehow the drill user has to be able to specify a path of multiple schema jar files, in order (the order can matter), and that has to end up on the class path of the drill bits so that this loadSchema call can work.

mbeckerle · 2024-05-09T19:50:48Z

.../src/main/java/org/apache/drill/exec/store/daffodil/schema/DaffodilDataProcessorFactory.java

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


Schema source files can be spread over multiple jars (sometimes dozens of files), and they contain relative paths within them that must resolve relative to that same classpath.

Note: This compilation really wants to happen once on a central node and the resulting binary schema file wants to be cached somewhere that gets copied out to the drill bit locations.

Right now this either loads a binary DFDL schema, which needs to happen on the drill bits, or it needs to compile a DFDL schema source (which needs to happen centrally) to create the binary DFDL schema, which then gets loaded by the drill bits.

This has to get refactored because the drill bits should never be compiling a DFDL schema. They should only be loading up pre-compiled binary schemas.

mbeckerle commented Apr 27, 2024

View reviewed changes

mbeckerle changed the title ~~Adding Daffodil to Drill as a 'contrib'~~ DRILL-2835: Adding Daffodil to Drill as a contrib. Apr 27, 2024

mbeckerle changed the title ~~DRILL-2835: Adding Daffodil to Drill as a contrib.~~ DRILL-8474: Adding Daffodil to Drill as a contrib. Apr 27, 2024

cgivre reviewed Apr 28, 2024

View reviewed changes

Changes to fix checkstyle errors

bf6d52c

mbeckerle commented May 9, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRILL-8474: Adding Daffodil to Drill as a contrib. #2909

DRILL-8474: Adding Daffodil to Drill as a contrib. #2909

mbeckerle commented Apr 27, 2024 •

edited

mbeckerle Apr 27, 2024

mbeckerle commented Apr 27, 2024

shfshihuafeng commented Apr 28, 2024 •

edited

cgivre Apr 28, 2024

cgivre Apr 28, 2024

mbeckerle Apr 28, 2024

cgivre Apr 28, 2024 •

edited

mbeckerle commented Apr 29, 2024

cgivre commented May 6, 2024 via email

mbeckerle commented May 6, 2024

mbeckerle May 9, 2024

mbeckerle May 9, 2024

mbeckerle May 9, 2024

mbeckerle May 9, 2024

DRILL-8474: Adding Daffodil to Drill as a contrib. #2909

Are you sure you want to change the base?

DRILL-8474: Adding Daffodil to Drill as a contrib. #2909

Conversation

mbeckerle commented Apr 27, 2024 • edited

DRILL-8474: Adding Daffodil to Drill as a contrib.

Description

Documentation

Testing

Choose a reason for hiding this comment

mbeckerle commented Apr 27, 2024

shfshihuafeng commented Apr 28, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

cgivre Apr 28, 2024 • edited

Choose a reason for hiding this comment

mbeckerle commented Apr 29, 2024

cgivre commented May 6, 2024 via email

mbeckerle commented May 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbeckerle commented Apr 27, 2024 •

edited

shfshihuafeng commented Apr 28, 2024 •

edited

cgivre Apr 28, 2024 •

edited