From d06a3e0ed08344ee0d8fd0396fcf379bf63cf5df Mon Sep 17 00:00:00 2001
From: Marius Grama <findinpath@gmail.com>
Date: Fri, 8 Sep 2023 22:27:51 +0200
Subject: [PATCH] Override default implementation of `skip(long)` method

Delegate to `stream` the implementation of `skip(long)` method.
By doing this, it is ensured that not the default implementation
from `java.io.InputStream` is being used, because it calls
`read()` causing actual reads from the underlying input stream
although `skip` is just a logical operation.

Co-authored-by: James Petty <pettja@amazon.com>
---
 .../AbstractTestTrinoFileSystem.java          | 47 ++++++++++++++-----
 .../filesystem/hdfs/HdfsTrinoInputStream.java | 13 +++++
 2 files changed, 47 insertions(+), 13 deletions(-)

diff --git a/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java b/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java
index 64e28235cedfd..c64dd60d6705e 100644
--- a/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java
+++ b/lib/trino-filesystem/src/test/java/io/trino/filesystem/AbstractTestTrinoFileSystem.java
@@ -254,7 +254,12 @@ public void testInputFile()
                 assertThat(inputStream.getPosition()).isEqualTo(fileSize);
                 assertThat(inputStream.read()).isLessThan(0);
                 assertThat(inputStream.read(bytes)).isLessThan(0);
-                assertThat(inputStream.skip(10)).isEqualTo(0);
+                if (seekPastEndOfFileFails()) {
+                    assertThat(inputStream.skip(10)).isEqualTo(0);
+                }
+                else {
+                    assertThat(inputStream.skip(10)).isEqualTo(10L);
+                }
 
                 // seek 4 MB in and read byte at a time
                 inputStream.seek(4 * MEGABYTE);
@@ -270,7 +275,12 @@ public void testInputFile()
                 assertThat(inputStream.getPosition()).isEqualTo(fileSize);
                 assertThat(inputStream.read()).isLessThan(0);
                 assertThat(inputStream.read(bytes)).isLessThan(0);
-                assertThat(inputStream.skip(10)).isEqualTo(0);
+                if (seekPastEndOfFileFails()) {
+                    assertThat(inputStream.skip(10)).isEqualTo(0);
+                }
+                else {
+                    assertThat(inputStream.skip(10)).isEqualTo(10L);
+                }
 
                 // seek 1MB at a time
                 for (int i = 0; i < 16; i++) {
@@ -297,9 +307,11 @@ public void testInputFile()
                     assertThat(slice.getInt(0)).isEqualTo(expectedPosition / 4);
                     expectedPosition += size;
                 }
-                long skipSize = inputStream.skip(MEGABYTE);
-                assertThat(skipSize).isEqualTo(fileSize - expectedPosition);
-                assertThat(inputStream.getPosition()).isEqualTo(fileSize);
+                if (seekPastEndOfFileFails()) {
+                    long skipSize = inputStream.skip(MEGABYTE);
+                    assertThat(skipSize).isEqualTo(fileSize - expectedPosition);
+                    assertThat(inputStream.getPosition()).isEqualTo(fileSize);
+                }
 
                 // skip N bytes
                 inputStream.seek(0);
@@ -318,18 +330,27 @@ public void testInputFile()
                 inputStream.skipNBytes(fileSize - expectedPosition);
                 assertThat(inputStream.getPosition()).isEqualTo(fileSize);
 
-                // skip beyond the end of the file is not allowed
-                inputStream.seek(expectedPosition);
-                assertThat(expectedPosition + MEGABYTE).isGreaterThan(fileSize);
-                assertThatThrownBy(() -> inputStream.skipNBytes(MEGABYTE))
-                        .isInstanceOf(EOFException.class);
+                if (seekPastEndOfFileFails()) {
+                    // skip beyond the end of the file is not allowed
+                    inputStream.seek(expectedPosition);
+                    assertThat(expectedPosition + MEGABYTE).isGreaterThan(fileSize);
+                    assertThatThrownBy(() -> inputStream.skipNBytes(MEGABYTE))
+                            .isInstanceOf(EOFException.class);
+                }
 
                 inputStream.seek(fileSize);
-                assertThatThrownBy(() -> inputStream.skipNBytes(1))
-                        .isInstanceOf(EOFException.class);
+                if (seekPastEndOfFileFails()) {
+                    assertThatThrownBy(() -> inputStream.skipNBytes(1))
+                            .isInstanceOf(EOFException.class);
+                }
 
                 inputStream.seek(fileSize);
-                assertThat(inputStream.skip(1)).isEqualTo(0);
+                if (seekPastEndOfFileFails()) {
+                    assertThat(inputStream.skip(1)).isEqualTo(0);
+                }
+                else {
+                    assertThat(inputStream.skip(1)).isEqualTo(1L);
+                }
 
                 // seek beyond the end of the file, is not allowed
                 long currentPosition = fileSize - 500;
diff --git a/lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsTrinoInputStream.java b/lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsTrinoInputStream.java
index 9f660ca3d1f64..8e1fa57b06e4b 100644
--- a/lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsTrinoInputStream.java
+++ b/lib/trino-hdfs/src/main/java/io/trino/filesystem/hdfs/HdfsTrinoInputStream.java
@@ -107,6 +107,19 @@ public int read(byte[] b, int off, int len)
         }
     }
 
+    @Override
+    public long skip(long n)
+            throws IOException
+    {
+        ensureOpen();
+        try {
+            return stream.skip(n);
+        }
+        catch (IOException e) {
+            throw new IOException("Skipping %s bytes of file %s failed: %s".formatted(n, location, e.getMessage()), e);
+        }
+    }
+
     @Override
     public void close()
             throws IOException