You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@drill.apache.org by GitBox <gi...@apache.org> on 2021/11/23 18:55:37 UTC

[GitHub] [drill] cgivre opened a new pull request #2386: DRILL-8054: Add SAS Format Plugin

cgivre opened a new pull request #2386:
URL: https://github.com/apache/drill/pull/2386


   # [DRILL-8054](https://issues.apache.org/jira/browse/DRILL-8054): Add SAS Format Plugin
   
   ## Description
   This format plugin adds the ability to read SAS files.  Once merged, Drill will be able to read the same file types as Pandas.
   
   ## Documentation
   Docs available in README.
   
   ## Testing
   Added unit tests.


-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756131357



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasFormatConfig.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.sas;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.FormatPluginConfig;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+@EqualsAndHashCode
+@ToString

Review comment:
       For this case, it doesn't really matter.   It can have a really bad effect on GroupScans and some of the schema planning objects.




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on pull request #2386:
URL: https://github.com/apache/drill/pull/2386#issuecomment-977915572


   @dzamo @luocooong 
   Thank you for the quick review.  I addressed all your review comments.  


-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] dzamo commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
dzamo commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r755985119



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasFormatConfig.java
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.sas;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.FormatPluginConfig;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+@JsonTypeName(SasFormatPlugin.DEFAULT_NAME)
+@JsonInclude(JsonInclude.Include.NON_DEFAULT)
+public class SasFormatConfig implements FormatPluginConfig {
+  private final List<String> extensions;
+
+  // Omitted properties take reasonable defaults
+  @JsonCreator
+  public SasFormatConfig(@JsonProperty("extensions") List<String> extensions) {
+    this.extensions = extensions == null ? Collections.singletonList("sas7bdat") : ImmutableList.copyOf(extensions);
+  }
+
+  @JsonInclude(JsonInclude.Include.NON_DEFAULT)
+  public List<String> getExtensions() {
+    return extensions;
+  }
+
+  @Override
+  public int hashCode() {

Review comment:
       I think hashCode and equals could be Lomboked if that's something we're defaulting to these days.

##########
File path: contrib/format-sas/README.md
##########
@@ -0,0 +1,34 @@
+# Format Plugin for SAS Files
+This format plugin enables Drill to read SAS files. (sas7bdat)
+
+## Data Types
+The SAS format supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` formats.

Review comment:
       ```suggestion
   The SAS format supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` types.
   ```




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756110206



##########
File path: contrib/format-sas/src/test/java/org/apache/drill/exec/store/sas/TestSasReader.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.sas;
+
+import org.apache.drill.categories.RowSetTests;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.rowSet.RowSet;
+import org.apache.drill.exec.physical.rowSet.RowSetBuilder;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.nio.file.Paths;
+import java.time.LocalDate;
+import java.time.LocalTime;
+
+import static org.junit.Assert.assertEquals;
+import static org.apache.drill.test.QueryTestUtil.generateCompressedFile;
+
+
+@Category(RowSetTests.class)
+public class TestSasReader extends ClusterTest {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher));
+
+    // Needed for compressed file unit test
+    dirTestWatcher.copyResourceToRoot(Paths.get("sas/"));
+  }
+
+  @Test
+  public void testStarQuery() throws Exception {
+    String sql = "SELECT * FROM cp.`sas/mixed_data_two.sas7bdat` WHERE x1 = 1";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("x1", MinorType.BIGINT)
+      .addNullable("x2", MinorType.FLOAT8)
+      .addNullable("x3", MinorType.VARCHAR)
+      .addNullable("x4", MinorType.FLOAT8)
+      .addNullable("x5", MinorType.FLOAT8)
+      .addNullable("x6", MinorType.FLOAT8)
+      .addNullable("x7", MinorType.FLOAT8)
+      .addNullable("x8", MinorType.FLOAT8)
+      .addNullable("x9", MinorType.FLOAT8)
+      .addNullable("x10", MinorType.FLOAT8)
+      .addNullable("x11", MinorType.FLOAT8)
+      .addNullable("x12", MinorType.FLOAT8)
+      .addNullable("x13", MinorType.FLOAT8)
+      .addNullable("x14", MinorType.FLOAT8)
+      .addNullable("x15", MinorType.BIGINT)
+      .addNullable("x16", MinorType.BIGINT)
+      .addNullable("x17", MinorType.BIGINT)
+      .addNullable("x18", MinorType.BIGINT)
+      .addNullable("x19", MinorType.BIGINT)
+      .addNullable("x20", MinorType.BIGINT)
+      .addNullable("x21", MinorType.BIGINT)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(1L, 1.1, "AAAAAAAA", 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 31626061L, 31625961L, 31627061L, 31616061L, 31636061L, 31526061L, 31726061L)
+      .addRow(1L, 1.1, "AAAAAAAA", 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 31626061L, 31625961L, 31627061L, 31616061L, 31636061L, 31526061L, 31726061L)
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testMetadataColumns() throws Exception {
+    String sql = "SELECT _compression_method, _file_label, _file_type, " +
+      "_os_name, _os_type, _sas_release, _session_encoding, _server_type, " +
+      "_date_created, _date_modified FROM cp.`sas/date_formats.sas7bdat`";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("_compression_method", MinorType.VARCHAR)
+      .addNullable("_file_label", MinorType.VARCHAR)
+      .addNullable("_file_type", MinorType.VARCHAR)
+      .addNullable("_os_name", MinorType.VARCHAR)
+      .addNullable("_os_type", MinorType.VARCHAR)
+      .addNullable("_sas_release", MinorType.VARCHAR)
+      .addNullable("_session_encoding", MinorType.VARCHAR)
+      .addNullable("_server_type", MinorType.VARCHAR)
+      .addNullable("_date_created", MinorType.DATE)
+      .addNullable("_date_modified", MinorType.DATE)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(null, "DATA", null, null, "9.0401M4", null, "X64_7PRO", null,
+        LocalDate.parse("2017-03-14"), LocalDate.parse("2017-03-14"))
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testCompressedFile() throws Exception {
+    generateCompressedFile("sas/mixed_data_two.sas7bdat", "zip", "sas/mixed_data_two.sas7bdat.zip" );
+
+    String sql = "SELECT x1, x2, x3 FROM dfs.`sas/mixed_data_two.sas7bdat.zip` WHERE x1 = 1";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("x1", MinorType.BIGINT)
+      .addNullable("x2", MinorType.FLOAT8)
+      .addNullable("x3", MinorType.VARCHAR)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(1L, 1.1, "AAAAAAAA")
+      .addRow(1L, 1.1, "AAAAAAAA")
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testDates() throws Exception {
+    String sql = "SELECT b8601da, e8601da, `date` FROM cp.`sas/date_formats.sas7bdat`";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("b8601da", MinorType.DATE)
+      .addNullable("e8601da", MinorType.DATE)
+      .addNullable("date", MinorType.DATE)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(LocalDate.parse("2017-03-14"), LocalDate.parse("2017-03-14"), LocalDate.parse("2017-03-14"))
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testTimes() throws Exception {
+    String sql = "SELECT * FROM cp.`sas/time_formats.sas7bdat`";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("E8601LZ", MinorType.TIME)
+      .addNullable("E8601TM", MinorType.TIME)
+      .addNullable("HHMM", MinorType.TIME)
+      .addNullable("HOUR", MinorType.TIME)
+      .addNullable("MMSS", MinorType.TIME)
+      .addNullable("TIME", MinorType.TIME)
+      .addNullable("TIMEAMPM", MinorType.TIME)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"),
+        LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"))
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testSerDe() throws Exception {
+    String sql = "SELECT COUNT(*) as cnt FROM cp.`sas/mixed_data_two.sas7bdat` ";
+    String plan = queryBuilder().sql(sql).explainJson();
+    long cnt = queryBuilder().physical(plan).singletonLong();
+    assertEquals("Counts should match",50L, cnt);

Review comment:
       Fixed




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] dzamo commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
dzamo commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r755999827



##########
File path: contrib/format-sas/README.md
##########
@@ -0,0 +1,34 @@
+# Format Plugin for SAS Files
+This format plugin enables Drill to read SAS files. (sas7bdat)
+
+## Data Types
+The SAS format supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` formats.

Review comment:
       ```suggestion
   The SAS format plugin supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` Drill data types.
   ```




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre merged pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre merged pull request #2386:
URL: https://github.com/apache/drill/pull/2386


   


-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756112189



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasBatchReader.java
##########
@@ -0,0 +1,454 @@
+/*
+ * 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.sas;
+
+import com.epam.parso.Column;
+import com.epam.parso.SasFileProperties;
+import com.epam.parso.SasFileReader;
+import com.epam.parso.impl.SasFileReaderImpl;
+import org.apache.drill.common.AutoCloseables;
+import org.apache.drill.common.exceptions.CustomErrorContext;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.types.TypeProtos.DataMode;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.impl.scan.file.FileScanFramework;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedReader;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.MaterializedField;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.MetadataUtils;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.hadoop.mapred.FileSplit;
+import org.apache.parquet.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.ZoneOffset;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+public class SasBatchReader implements ManagedReader<FileScanFramework.FileSchemaNegotiator> {
+  private static final Logger logger = LoggerFactory.getLogger(SasBatchReader.class);
+  private final int maxRecords;
+  private final List<SasColumnWriter> writerList;
+  private FileSplit split;
+  private InputStream fsStream;
+  private SasFileReader sasFileReader;
+  private CustomErrorContext errorContext;
+  private RowSetLoader rowWriter;
+  private Object[] firstRow;
+
+
+  private String compressionMethod;
+  private String fileLabel;
+  private String fileType;
+  private String osName;
+  private String osType;
+  private String sasRelease;
+  private String sessionEncoding;
+  private String serverType;
+  private LocalDate dateCreated;
+  private LocalDate dateModified;
+
+  private enum IMPLICIT_STRING_COLUMN {
+    COMPRESSION_METHOD("_compression_method"),
+    ENCODING("_encoding"),
+    FILE_LABEL("_file_label"),
+    FILE_TYPE("_file_type"),
+    OS_NAME("_os_name"),
+    OS_TYPE("_os_type"),
+    SAS_RELEASE("_sas_release"),
+    SESSION_ENCODING("_session_encoding");
+
+    private final String fieldName;
+
+    IMPLICIT_STRING_COLUMN(String fieldName) {
+      this.fieldName = fieldName;
+    }
+
+    public String getFieldName() {
+      return fieldName;
+    }
+  }
+
+  private enum IMPLICIT_DATE_COLUMN {
+    CREATED_DATE("_date_created"),
+    MODIFIED_DATE("_date_modified");
+
+    private final String fieldName;
+
+    IMPLICIT_DATE_COLUMN(String fieldName) {
+      this.fieldName = fieldName;
+    }
+
+    public String getFieldName() {
+      return fieldName;
+    }
+  }
+
+  public static class SasReaderConfig {
+    protected final SasFormatPlugin plugin;
+    public SasReaderConfig(SasFormatPlugin plugin) {
+      this.plugin = plugin;
+    }
+  }
+
+  public SasBatchReader(int maxRecords) {
+    this.maxRecords = maxRecords;
+    writerList = new ArrayList<>();
+  }
+
+  @Override
+  public boolean open(FileScanFramework.FileSchemaNegotiator negotiator) {
+    split = negotiator.split();
+    errorContext = negotiator.parentErrorContext();
+    openFile(negotiator);
+
+    TupleMetadata schema;
+    if (negotiator.hasProvidedSchema()) {
+      schema = negotiator.providedSchema();
+    } else {
+      schema = buildSchema();
+    }
+    schema = addImplicitColumnsToSchema(schema);
+    negotiator.tableSchema(schema, true);
+
+    ResultSetLoader loader = negotiator.build();
+    rowWriter = loader.writer();
+    buildWriterList(schema);
+
+    return true;
+  }
+
+  private void openFile(FileScanFramework.FileSchemaNegotiator negotiator) {
+    try {
+      fsStream = negotiator.fileSystem().openPossiblyCompressedStream(split.getPath());
+      sasFileReader = new SasFileReaderImpl(fsStream);
+      firstRow = sasFileReader.readNext();
+    } catch (IOException e) {
+      throw UserException
+        .dataReadError(e)
+        .message("Unable to open SAS File %s", split.getPath())
+        .addContext(e.getMessage())
+        .addContext(errorContext)
+        .build(logger);
+    }
+  }
+
+  private TupleMetadata buildSchema() {
+    SchemaBuilder builder = new SchemaBuilder();
+    List<Column> columns = sasFileReader.getColumns();
+    int counter = 0;
+    for (Column column : columns) {
+      String fieldName = column.getName();
+      try {
+        MinorType type = getType(firstRow[counter].getClass().getSimpleName());
+        if (type == MinorType.BIGINT && !column.getFormat().isEmpty()) {
+          logger.debug("Found possible time");
+          type = MinorType.TIME;
+        }
+        builder.addNullable(fieldName, type);
+      } catch (Exception e) {
+        System.out.println("Error with column type: " + firstRow[counter].getClass().getSimpleName());

Review comment:
       Replaced with `UserException`.




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756107405



##########
File path: contrib/format-sas/README.md
##########
@@ -0,0 +1,34 @@
+# Format Plugin for SAS Files
+This format plugin enables Drill to read SAS files. (sas7bdat)
+
+## Data Types
+The SAS format supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` formats.

Review comment:
       Fixed.




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] dzamo commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
dzamo commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756135990



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasFormatConfig.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.sas;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.FormatPluginConfig;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+@EqualsAndHashCode
+@ToString

Review comment:
       Let's merge?




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] luocooong commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
luocooong commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r755876354



##########
File path: contrib/format-sas/README.md
##########
@@ -0,0 +1,34 @@
+# Format Plugin for SAS Files
+This format plugin enables Drill to read SAS files. (sas7bdat)
+
+## Data Types
+The SAS format supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` formats.
+
+## Implicit Fields (Metadata)
+The SAS reader provides the following file metadata fields:
+* `_compression_method`
+* `_encoding`
+* `_file_label`
+* `_file_type`
+* `_os_name`
+* `_os_type`
+* `_sas_release`
+* `_session_encoding`
+* `_date_created`
+* `_date_modified`
+
+## Schema Provisioning
+Drill will infer the schema of your data.  
+
+## Configuration Options 
+This function has no configuration options other than the file extension.
+
+```json
+  "sas": {
+  "type": "sas",
+  "extensions": [
+    "sas7bdat"
+  ]
+}
+```
+This plugin is enabled by default. 

Review comment:
       enabled ?

##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasBatchReader.java
##########
@@ -0,0 +1,454 @@
+/*
+ * 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.sas;
+
+import com.epam.parso.Column;
+import com.epam.parso.SasFileProperties;
+import com.epam.parso.SasFileReader;
+import com.epam.parso.impl.SasFileReaderImpl;
+import org.apache.drill.common.AutoCloseables;
+import org.apache.drill.common.exceptions.CustomErrorContext;
+import org.apache.drill.common.exceptions.UserException;
+import org.apache.drill.common.types.TypeProtos.DataMode;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.impl.scan.file.FileScanFramework;
+import org.apache.drill.exec.physical.impl.scan.framework.ManagedReader;
+import org.apache.drill.exec.physical.resultSet.ResultSetLoader;
+import org.apache.drill.exec.physical.resultSet.RowSetLoader;
+import org.apache.drill.exec.record.MaterializedField;
+import org.apache.drill.exec.record.metadata.ColumnMetadata;
+import org.apache.drill.exec.record.metadata.MetadataUtils;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.exec.vector.accessor.ScalarWriter;
+import org.apache.hadoop.mapred.FileSplit;
+import org.apache.parquet.Strings;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+import java.io.IOException;
+import java.io.InputStream;
+import java.time.Instant;
+import java.time.LocalDate;
+import java.time.LocalTime;
+import java.time.ZoneOffset;
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.List;
+
+public class SasBatchReader implements ManagedReader<FileScanFramework.FileSchemaNegotiator> {
+  private static final Logger logger = LoggerFactory.getLogger(SasBatchReader.class);
+  private final int maxRecords;
+  private final List<SasColumnWriter> writerList;
+  private FileSplit split;
+  private InputStream fsStream;
+  private SasFileReader sasFileReader;
+  private CustomErrorContext errorContext;
+  private RowSetLoader rowWriter;
+  private Object[] firstRow;
+
+
+  private String compressionMethod;
+  private String fileLabel;
+  private String fileType;
+  private String osName;
+  private String osType;
+  private String sasRelease;
+  private String sessionEncoding;
+  private String serverType;
+  private LocalDate dateCreated;
+  private LocalDate dateModified;
+
+  private enum IMPLICIT_STRING_COLUMN {
+    COMPRESSION_METHOD("_compression_method"),
+    ENCODING("_encoding"),
+    FILE_LABEL("_file_label"),
+    FILE_TYPE("_file_type"),
+    OS_NAME("_os_name"),
+    OS_TYPE("_os_type"),
+    SAS_RELEASE("_sas_release"),
+    SESSION_ENCODING("_session_encoding");
+
+    private final String fieldName;
+
+    IMPLICIT_STRING_COLUMN(String fieldName) {
+      this.fieldName = fieldName;
+    }
+
+    public String getFieldName() {
+      return fieldName;
+    }
+  }
+
+  private enum IMPLICIT_DATE_COLUMN {
+    CREATED_DATE("_date_created"),
+    MODIFIED_DATE("_date_modified");
+
+    private final String fieldName;
+
+    IMPLICIT_DATE_COLUMN(String fieldName) {
+      this.fieldName = fieldName;
+    }
+
+    public String getFieldName() {
+      return fieldName;
+    }
+  }
+
+  public static class SasReaderConfig {
+    protected final SasFormatPlugin plugin;
+    public SasReaderConfig(SasFormatPlugin plugin) {
+      this.plugin = plugin;
+    }
+  }
+
+  public SasBatchReader(int maxRecords) {
+    this.maxRecords = maxRecords;
+    writerList = new ArrayList<>();
+  }
+
+  @Override
+  public boolean open(FileScanFramework.FileSchemaNegotiator negotiator) {
+    split = negotiator.split();
+    errorContext = negotiator.parentErrorContext();
+    openFile(negotiator);
+
+    TupleMetadata schema;
+    if (negotiator.hasProvidedSchema()) {
+      schema = negotiator.providedSchema();
+    } else {
+      schema = buildSchema();
+    }
+    schema = addImplicitColumnsToSchema(schema);
+    negotiator.tableSchema(schema, true);
+
+    ResultSetLoader loader = negotiator.build();
+    rowWriter = loader.writer();
+    buildWriterList(schema);
+
+    return true;
+  }
+
+  private void openFile(FileScanFramework.FileSchemaNegotiator negotiator) {
+    try {
+      fsStream = negotiator.fileSystem().openPossiblyCompressedStream(split.getPath());
+      sasFileReader = new SasFileReaderImpl(fsStream);
+      firstRow = sasFileReader.readNext();
+    } catch (IOException e) {
+      throw UserException
+        .dataReadError(e)
+        .message("Unable to open SAS File %s", split.getPath())
+        .addContext(e.getMessage())
+        .addContext(errorContext)
+        .build(logger);
+    }
+  }
+
+  private TupleMetadata buildSchema() {
+    SchemaBuilder builder = new SchemaBuilder();
+    List<Column> columns = sasFileReader.getColumns();
+    int counter = 0;
+    for (Column column : columns) {
+      String fieldName = column.getName();
+      try {
+        MinorType type = getType(firstRow[counter].getClass().getSimpleName());
+        if (type == MinorType.BIGINT && !column.getFormat().isEmpty()) {
+          logger.debug("Found possible time");
+          type = MinorType.TIME;
+        }
+        builder.addNullable(fieldName, type);
+      } catch (Exception e) {
+        System.out.println("Error with column type: " + firstRow[counter].getClass().getSimpleName());

Review comment:
       Replace to logger.error();

##########
File path: contrib/format-sas/src/test/java/org/apache/drill/exec/store/sas/TestSasReader.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.sas;
+
+import org.apache.drill.categories.RowSetTests;
+import org.apache.drill.common.types.TypeProtos.MinorType;
+import org.apache.drill.exec.physical.rowSet.RowSet;
+import org.apache.drill.exec.physical.rowSet.RowSetBuilder;
+import org.apache.drill.exec.record.metadata.SchemaBuilder;
+import org.apache.drill.exec.record.metadata.TupleMetadata;
+import org.apache.drill.test.ClusterFixture;
+import org.apache.drill.test.ClusterTest;
+import org.apache.drill.test.rowSet.RowSetComparison;
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.experimental.categories.Category;
+
+import java.nio.file.Paths;
+import java.time.LocalDate;
+import java.time.LocalTime;
+
+import static org.junit.Assert.assertEquals;
+import static org.apache.drill.test.QueryTestUtil.generateCompressedFile;
+
+
+@Category(RowSetTests.class)
+public class TestSasReader extends ClusterTest {
+
+  @BeforeClass
+  public static void setup() throws Exception {
+    ClusterTest.startCluster(ClusterFixture.builder(dirTestWatcher));
+
+    // Needed for compressed file unit test
+    dirTestWatcher.copyResourceToRoot(Paths.get("sas/"));
+  }
+
+  @Test
+  public void testStarQuery() throws Exception {
+    String sql = "SELECT * FROM cp.`sas/mixed_data_two.sas7bdat` WHERE x1 = 1";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("x1", MinorType.BIGINT)
+      .addNullable("x2", MinorType.FLOAT8)
+      .addNullable("x3", MinorType.VARCHAR)
+      .addNullable("x4", MinorType.FLOAT8)
+      .addNullable("x5", MinorType.FLOAT8)
+      .addNullable("x6", MinorType.FLOAT8)
+      .addNullable("x7", MinorType.FLOAT8)
+      .addNullable("x8", MinorType.FLOAT8)
+      .addNullable("x9", MinorType.FLOAT8)
+      .addNullable("x10", MinorType.FLOAT8)
+      .addNullable("x11", MinorType.FLOAT8)
+      .addNullable("x12", MinorType.FLOAT8)
+      .addNullable("x13", MinorType.FLOAT8)
+      .addNullable("x14", MinorType.FLOAT8)
+      .addNullable("x15", MinorType.BIGINT)
+      .addNullable("x16", MinorType.BIGINT)
+      .addNullable("x17", MinorType.BIGINT)
+      .addNullable("x18", MinorType.BIGINT)
+      .addNullable("x19", MinorType.BIGINT)
+      .addNullable("x20", MinorType.BIGINT)
+      .addNullable("x21", MinorType.BIGINT)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(1L, 1.1, "AAAAAAAA", 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 31626061L, 31625961L, 31627061L, 31616061L, 31636061L, 31526061L, 31726061L)
+      .addRow(1L, 1.1, "AAAAAAAA", 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 1.1, 31626061L, 31625961L, 31627061L, 31616061L, 31636061L, 31526061L, 31726061L)
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testMetadataColumns() throws Exception {
+    String sql = "SELECT _compression_method, _file_label, _file_type, " +
+      "_os_name, _os_type, _sas_release, _session_encoding, _server_type, " +
+      "_date_created, _date_modified FROM cp.`sas/date_formats.sas7bdat`";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("_compression_method", MinorType.VARCHAR)
+      .addNullable("_file_label", MinorType.VARCHAR)
+      .addNullable("_file_type", MinorType.VARCHAR)
+      .addNullable("_os_name", MinorType.VARCHAR)
+      .addNullable("_os_type", MinorType.VARCHAR)
+      .addNullable("_sas_release", MinorType.VARCHAR)
+      .addNullable("_session_encoding", MinorType.VARCHAR)
+      .addNullable("_server_type", MinorType.VARCHAR)
+      .addNullable("_date_created", MinorType.DATE)
+      .addNullable("_date_modified", MinorType.DATE)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(null, "DATA", null, null, "9.0401M4", null, "X64_7PRO", null,
+        LocalDate.parse("2017-03-14"), LocalDate.parse("2017-03-14"))
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testCompressedFile() throws Exception {
+    generateCompressedFile("sas/mixed_data_two.sas7bdat", "zip", "sas/mixed_data_two.sas7bdat.zip" );
+
+    String sql = "SELECT x1, x2, x3 FROM dfs.`sas/mixed_data_two.sas7bdat.zip` WHERE x1 = 1";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("x1", MinorType.BIGINT)
+      .addNullable("x2", MinorType.FLOAT8)
+      .addNullable("x3", MinorType.VARCHAR)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(1L, 1.1, "AAAAAAAA")
+      .addRow(1L, 1.1, "AAAAAAAA")
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testDates() throws Exception {
+    String sql = "SELECT b8601da, e8601da, `date` FROM cp.`sas/date_formats.sas7bdat`";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("b8601da", MinorType.DATE)
+      .addNullable("e8601da", MinorType.DATE)
+      .addNullable("date", MinorType.DATE)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(LocalDate.parse("2017-03-14"), LocalDate.parse("2017-03-14"), LocalDate.parse("2017-03-14"))
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testTimes() throws Exception {
+    String sql = "SELECT * FROM cp.`sas/time_formats.sas7bdat`";
+    RowSet results  = client.queryBuilder().sql(sql).rowSet();
+
+    TupleMetadata expectedSchema = new SchemaBuilder()
+      .addNullable("E8601LZ", MinorType.TIME)
+      .addNullable("E8601TM", MinorType.TIME)
+      .addNullable("HHMM", MinorType.TIME)
+      .addNullable("HOUR", MinorType.TIME)
+      .addNullable("MMSS", MinorType.TIME)
+      .addNullable("TIME", MinorType.TIME)
+      .addNullable("TIMEAMPM", MinorType.TIME)
+      .buildSchema();
+
+    RowSet expected = new RowSetBuilder(client.allocator(), expectedSchema)
+      .addRow(LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"),
+        LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"), LocalTime.parse("10:10:10"))
+      .build();
+
+    new RowSetComparison(expected).verifyAndClearAll(results);
+  }
+
+  @Test
+  public void testSerDe() throws Exception {
+    String sql = "SELECT COUNT(*) as cnt FROM cp.`sas/mixed_data_two.sas7bdat` ";
+    String plan = queryBuilder().sql(sql).explainJson();
+    long cnt = queryBuilder().physical(plan).singletonLong();
+    assertEquals("Counts should match",50L, cnt);

Review comment:
       Add a space in front of "50L".




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756043103



##########
File path: contrib/format-sas/README.md
##########
@@ -0,0 +1,34 @@
+# Format Plugin for SAS Files
+This format plugin enables Drill to read SAS files. (sas7bdat)
+
+## Data Types
+The SAS format supports the `VARCHAR`, `LONG`, `DOUBLE` and `DATE` formats.
+
+## Implicit Fields (Metadata)
+The SAS reader provides the following file metadata fields:
+* `_compression_method`
+* `_encoding`
+* `_file_label`
+* `_file_type`
+* `_os_name`
+* `_os_type`
+* `_sas_release`
+* `_session_encoding`
+* `_date_created`
+* `_date_modified`
+
+## Schema Provisioning
+Drill will infer the schema of your data.  
+
+## Configuration Options 
+This function has no configuration options other than the file extension.
+
+```json
+  "sas": {
+  "type": "sas",
+  "extensions": [
+    "sas7bdat"
+  ]
+}
+```
+This plugin is enabled by default. 

Review comment:
       @luocooong Thanks for the review!  Format plugins don't have to have the `enabled` parameter in them.  Simply by including them in the configuration, they are enabled.




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756109720



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasFormatConfig.java
##########
@@ -0,0 +1,73 @@
+/*
+ * 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.sas;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.FormatPluginConfig;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+@JsonTypeName(SasFormatPlugin.DEFAULT_NAME)
+@JsonInclude(JsonInclude.Include.NON_DEFAULT)
+public class SasFormatConfig implements FormatPluginConfig {
+  private final List<String> extensions;
+
+  // Omitted properties take reasonable defaults
+  @JsonCreator
+  public SasFormatConfig(@JsonProperty("extensions") List<String> extensions) {
+    this.extensions = extensions == null ? Collections.singletonList("sas7bdat") : ImmutableList.copyOf(extensions);
+  }
+
+  @JsonInclude(JsonInclude.Include.NON_DEFAULT)
+  public List<String> getExtensions() {
+    return extensions;
+  }
+
+  @Override
+  public int hashCode() {

Review comment:
       Good point.  I lomboked the `toString` as well. 




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] dzamo commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
dzamo commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756126200



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasFormatConfig.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.sas;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.FormatPluginConfig;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+@EqualsAndHashCode
+@ToString

Review comment:
       @cgivre toString was one I wasn't sure should be lomboked because we have that `PlanStringBuilder` which I think does some special formatting for display in query plans... 




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [drill] cgivre commented on a change in pull request #2386: DRILL-8054: Add SAS Format Plugin

Posted by GitBox <gi...@apache.org>.
cgivre commented on a change in pull request #2386:
URL: https://github.com/apache/drill/pull/2386#discussion_r756136961



##########
File path: contrib/format-sas/src/main/java/org/apache/drill/exec/store/sas/SasFormatConfig.java
##########
@@ -0,0 +1,54 @@
+/*
+ * 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.sas;
+
+import com.fasterxml.jackson.annotation.JsonCreator;
+import com.fasterxml.jackson.annotation.JsonInclude;
+import com.fasterxml.jackson.annotation.JsonProperty;
+import com.fasterxml.jackson.annotation.JsonTypeName;
+
+import lombok.EqualsAndHashCode;
+import lombok.ToString;
+import org.apache.drill.common.PlanStringBuilder;
+import org.apache.drill.common.logical.FormatPluginConfig;
+import org.apache.drill.shaded.guava.com.google.common.collect.ImmutableList;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Objects;
+
+@EqualsAndHashCode
+@ToString

Review comment:
       Sure..  I'll wait until the CI passes, then merge.




-- 
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-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org