HADOOP-18830. Reviews and test completion

Change-Id: I72fda6324d04856c2f2016320f507f12e7efba0c

Author: steveloughran

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/ChangeTracker.java

@@ -223,7 +223,7 @@ public void processResponse(final CopyObjectResponse copyObjectResponse)
    * cause.
    * @param e the exception
    * @param operation the operation performed when the exception was
-   * generated (e.g. "copy", "read", "select").
+   * generated (e.g. "copy", "read").
    * @throws RemoteFileChangedException if the remote file has changed.
    */
   public void processException(SdkException e, String operation) throws

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/impl/OpenFileSupport.java

@@ -36,6 +36,7 @@
 import org.apache.hadoop.fs.s3a.S3ALocatedFileStatus;
 import org.apache.hadoop.fs.s3a.S3AReadOpContext;
 import org.apache.hadoop.fs.s3a.select.SelectConstants;
+import org.apache.hadoop.fs.store.LogExactlyOnce;
 
 import static org.apache.hadoop.fs.Options.OpenFileOptions.FS_OPTION_OPENFILE_BUFFER_SIZE;
 import static org.apache.hadoop.fs.Options.OpenFileOptions.FS_OPTION_OPENFILE_LENGTH;
@@ -67,6 +68,7 @@ public class OpenFileSupport {
   private static final Logger LOG =
       LoggerFactory.getLogger(OpenFileSupport.class);
 
+  public static final LogExactlyOnce LOG_NO_SQL_SELECT = new LogExactlyOnce(LOG);
   /**
    * For use when a value of an split/file length is unknown.
    */
@@ -152,6 +154,7 @@ public S3AReadOpContext applyDefaultOptions(S3AReadOpContext roc) {
 
   /**
    * Prepare to open a file from the openFile parameters.
+   * S3Select SQL is rejected if a mandatory opt, ignored if optional.
    * @param path path to the file
    * @param parameters open file parameters from the builder.
    * @param blockSize for fileStatus
@@ -167,10 +170,15 @@ public OpenFileInformation prepareToOpenFile(
       final long blockSize) throws IOException {
     Configuration options = parameters.getOptions();
     Set<String> mandatoryKeys = parameters.getMandatoryKeys();
-    // S3 Select is not supported in this release; fail wit
-    // a specific message.
+    // S3 Select is not supported in this release
     if (options.get(SelectConstants.SELECT_SQL, null) != null) {
-      throw new UnsupportedOperationException(SelectConstants.SELECT_UNSUPPORTED);
+      if (mandatoryKeys.contains(SelectConstants.SELECT_SQL)) {
+        // mandatory option: fail with a specific message.
+        throw new UnsupportedOperationException(SelectConstants.SELECT_UNSUPPORTED);
+      } else {
+        // optional; log once and continue
+        LOG_NO_SQL_SELECT.warn(SelectConstants.SELECT_UNSUPPORTED);
+      }
     }
     // choice of keys depends on open type
     rejectUnknownMandatoryKeys(

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/select/BlockingEnumeration.java

@@ -18,9 +18,11 @@
 
 /**
  * Was the location for support for S3 Select.
- * Now removed apart from some constants and a
- * class {@link org.apache.hadoop.fs.s3a.select.BlockingEnumeration}
- * which may be useful in future.
+ * Now removed apart from some constants.f
+ * There was a class {@code BlockingEnumeration} which
+ * mapped SdkPublisher to an Enumeration.
+ * This may be of use elsewhere; it can be retrieved from
+ * hadoop commit 8bf72346a59c.
  */
 @InterfaceAudience.Private
 @InterfaceStability.Unstable

hadoop-tools/hadoop-aws/src/main/java/org/apache/hadoop/fs/s3a/select/package-info.java

@@ -14,13 +14,14 @@
 
 # S3 Select
 
-<!-- MACRO{toc|fromDepth=0|toDepth=5} -->
-
 S3 Select is a feature for Amazon S3 introduced in April 2018. It allows for
 SQL-like SELECT expressions to be applied to files in some structured
 formats, including CSV and JSON.
 
 It is no longer supported in Hadoop releases.
 
 Any Hadoop release built on the [AWS V2 SDK](./aws_sdk_upgrade.html)
-will reject calls to open files using the select APIs.
+will reject calls to open files using the select APIs.
+
+If a build of Hadoop with S3 Select is desired, the relevant
+classes can be found in hadoop trunk commit `8bf72346a59c`.

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/s3_select.md

@@ -572,10 +572,6 @@ on third party stores.
     <name>test.fs.s3a.create.storage.class.enabled</name>
     <value>false</value>
   </property>
-  <property>
-    <name>fs.s3a.select.enabled</name>
-    <value>false</value>
-  </property>
   <property>
     <name>test.fs.s3a.sts.enabled</name>
     <value>false</value>

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/testing.md

@@ -419,11 +419,6 @@ this makes renaming and deleting significantly slower.
     <value>false</value>
   </property>
 
-  <property>
-    <name>fs.s3a.bucket.gcs-container.select.enabled</name>
-    <value>false</value>
-  </property>
-
   <property>
     <name>fs.s3a.bucket.gcs-container.path.style.access</name>
     <value>true</value>

hadoop-tools/hadoop-aws/src/site/markdown/tools/hadoop-aws/third_party_stores.md

@@ -21,45 +21,63 @@
 import org.assertj.core.api.Assertions;
 import org.junit.Test;
 
+import org.apache.hadoop.fs.Path;
+import org.apache.hadoop.fs.contract.ContractTestUtils;
 import org.apache.hadoop.fs.s3a.AbstractS3ATestBase;
+import org.apache.hadoop.fs.s3a.S3AFileSystem;
 import org.apache.hadoop.fs.s3a.s3guard.S3GuardTool;
 import org.apache.hadoop.util.ExitUtil;
 
 import static org.apache.hadoop.fs.s3a.select.SelectConstants.SELECT_SQL;
 import static org.apache.hadoop.fs.s3a.select.SelectConstants.SELECT_UNSUPPORTED;
 import static org.apache.hadoop.service.launcher.LauncherExitCodes.EXIT_UNSUPPORTED_VERSION;
 import static org.apache.hadoop.test.LambdaTestUtils.intercept;
+import static org.apache.hadoop.util.ExitUtil.disableSystemExit;
 
 /**
  * Verify that s3 select is unsupported.
  */
 public class ITestSelectUnsupported extends AbstractS3ATestBase {
 
+  /**
+   * S3 Select SQL statement.
+   */
+  private static final String STATEMENT = "SELECT *" +
+      " FROM S3Object s" +
+      " WHERE s._1 = 'foo'";
+
+  /**
+   * A {@code .must(SELECT_SQL, _)} option MSUT not fail the build.
+   */
   @Test
-  public void testS3GuardOpenFileMust() throws Throwable {
+  public void testSelectOpenFileMust() throws Throwable {
 
-    // TODO: intercept
-    getFileSystem().openFile(methodPath())
-        .must(SELECT_SQL, "SELECT *" +
-            " FROM S3Object s" +
-            " WHERE s._1 = 'foo'")
-        .build().get();
+    intercept(UnsupportedOperationException.class, SELECT_UNSUPPORTED, () ->
+        getFileSystem().openFile(methodPath())
+            .must(SELECT_SQL, STATEMENT)
+            .build()
+            /**/.get());
   }
 
+  /**
+   * A {@code .opt(SELECT_SQL, _)} option does not fail the build.
+   */
   @Test
-  public void testS3GuardOpenFileMay() throws Throwable {
-
-    // TODO: intercept
+  public void testSelectOpenFileMay() throws Throwable {
 
-    getFileSystem().openFile(methodPath())
-        .opt(SELECT_SQL, "SELECT *" +
-            " FROM S3Object s" +
-            " WHERE s._1 = 'foo'")
-        .build().get();
+    final Path path = methodPath();
+    final S3AFileSystem fs = getFileSystem();
+    ContractTestUtils.touch(fs, path);
+    fs.openFile(path)
+        .opt(SELECT_SQL, STATEMENT)
+        .build()
+        .get()
+        .close();
   }
 
   @Test
-  public void testFSLacksSupport() throws Throwable {
+  public void testPathCapabilityNotAvailable() throws Throwable {
+    describe("verify that the FS lacks the path capability");
     Assertions.assertThat(getFileSystem().hasPathCapability(methodPath(), SELECT_SQL))
         .describedAs("S3 Select reported as present")
         .isFalse();
@@ -68,15 +86,15 @@ public void testFSLacksSupport() throws Throwable {
   @Test
   public void testS3GuardToolFails() throws Throwable {
 
+    // ensure that the command doesn't actually exit the VM.
+    disableSystemExit();
     final ExitUtil.ExitException ex =
         intercept(ExitUtil.ExitException.class, SELECT_UNSUPPORTED,
             () -> S3GuardTool.main(new String[]{
-                "select",
-                "-sql", "SELECT * FROM S3Object s WHERE s._1 = 'foo'"
+                "select", "-sql", STATEMENT
             }));
     Assertions.assertThat(ex.getExitCode())
         .describedAs("exit code of exception")
         .isEqualTo(EXIT_UNSUPPORTED_VERSION);
-
   }
 }