Merge pull request #66628 from ClickHouse/fix-idiotic-code

Fix bad code: it was catching exceptions

src/Disks/IO/AsynchronousBoundedReadBuffer.h

@@ -34,7 +34,7 @@ public:
 
     String getFileName() const override { return impl->getFileName(); }
 
-    size_t getFileSize() override { return impl->getFileSize(); }
+    std::optional<size_t> tryGetFileSize() override { return impl->tryGetFileSize(); }
 
     String getInfoForLog() override { return impl->getInfoForLog(); }
 

src/Disks/IO/ReadBufferFromAzureBlobStorage.cpp

@@ -253,16 +253,15 @@ void ReadBufferFromAzureBlobStorage::initialize()
     initialized = true;
 }
 
-size_t ReadBufferFromAzureBlobStorage::getFileSize()
+std::optional<size_t> ReadBufferFromAzureBlobStorage::tryGetFileSize()
 {
     if (!blob_client)
         blob_client = std::make_unique<Azure::Storage::Blobs::BlobClient>(blob_container_client->GetBlobClient(path));
 
-    if (file_size.has_value())
-        return *file_size;
+    if (!file_size)
+        file_size = blob_client->GetProperties().Value.BlobSize;
 
-    file_size = blob_client->GetProperties().Value.BlobSize;
-    return *file_size;
+    return file_size;
 }
 
 size_t ReadBufferFromAzureBlobStorage::readBigAt(char * to, size_t n, size_t range_begin, const std::function<bool(size_t)> & /*progress_callback*/) const

src/Disks/IO/ReadBufferFromAzureBlobStorage.h

@@ -42,7 +42,7 @@ public:
 
     bool supportsRightBoundedReads() const override { return true; }
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     size_t readBigAt(char * to, size_t n, size_t range_begin, const std::function<bool(size_t)> & progress_callback) const override;
 

src/Disks/IO/ReadBufferFromRemoteFSGather.h

@@ -41,7 +41,7 @@ public:
 
     void setReadUntilEnd() override { setReadUntilPosition(getFileSize()); }
 
-    size_t getFileSize() override { return getTotalSize(blobs_to_read); }
+    std::optional<size_t> tryGetFileSize() override { return getTotalSize(blobs_to_read); }
 
     size_t getFileOffsetOfBufferEnd() const override { return file_offset_of_buffer_end; }
 

src/IO/Archives/LibArchiveReader.cpp

@@ -321,7 +321,7 @@ public:
     off_t getPosition() override { throw Exception(ErrorCodes::UNSUPPORTED_METHOD, "getPosition not supported when reading from archive"); }
     String getFileName() const override { return handle.getFileName(); }
 
-    size_t getFileSize() override { return handle.getFileInfo().uncompressed_size; }
+    std::optional<size_t> tryGetFileSize() override { return handle.getFileInfo().uncompressed_size; }
 
     Handle releaseHandle() && { return std::move(handle); }
 

src/IO/Archives/ZipArchiveReader.cpp

@@ -317,7 +317,7 @@ public:
 
     String getFileName() const override { return handle.getFileName(); }
 
-    size_t getFileSize() override { return handle.getFileInfo().uncompressed_size; }
+    std::optional<size_t> tryGetFileSize() override { return handle.getFileInfo().uncompressed_size; }
 
     /// Releases owned handle to pass it to an enumerator.
     HandleHolder releaseHandle() &&

src/IO/AsynchronousReadBufferFromFileDescriptor.cpp

@@ -244,7 +244,7 @@ void AsynchronousReadBufferFromFileDescriptor::rewind()
     file_offset_of_buffer_end = 0;
 }
 
-size_t AsynchronousReadBufferFromFileDescriptor::getFileSize()
+std::optional<size_t> AsynchronousReadBufferFromFileDescriptor::tryGetFileSize()
 {
     return getSizeFromFileDescriptor(fd, getFileName());
 }

src/IO/AsynchronousReadBufferFromFileDescriptor.h

@@ -68,7 +68,7 @@ public:
     /// Seek to the beginning, discarding already read data if any. Useful to reread file that changes on every read.
     void rewind();
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     size_t getFileOffsetOfBufferEnd() const override { return file_offset_of_buffer_end; }
 

src/IO/ConcatSeekableReadBuffer.h

@@ -21,7 +21,7 @@ public:
     off_t seek(off_t off, int whence) override;
     off_t getPosition() override;
 
-    size_t getFileSize() override { return total_size; }
+    std::optional<size_t> tryGetFileSize() override { return total_size; }
 
 private:
     bool nextImpl() override;

src/IO/MMapReadBufferFromFileDescriptor.cpp

@@ -87,7 +87,7 @@ off_t MMapReadBufferFromFileDescriptor::seek(off_t offset, int whence)
     return new_pos;
 }
 
-size_t MMapReadBufferFromFileDescriptor::getFileSize()
+std::optional<size_t> MMapReadBufferFromFileDescriptor::tryGetFileSize()
 {
     return getSizeFromFileDescriptor(getFD(), getFileName());
 }

src/IO/MMapReadBufferFromFileDescriptor.h

@@ -38,7 +38,7 @@ public:
 
     int getFD() const;
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     size_t readBigAt(char * to, size_t n, size_t offset, const std::function<bool(size_t)> &) const override;
     bool supportsReadAt() override { return true; }

src/IO/ParallelReadBuffer.cpp

@@ -152,7 +152,7 @@ off_t ParallelReadBuffer::seek(off_t offset, int whence)
     return offset;
 }
 
-size_t ParallelReadBuffer::getFileSize()
+std::optional<size_t> ParallelReadBuffer::tryGetFileSize()
 {
     return file_size;
 }

src/IO/ParallelReadBuffer.h

@@ -33,7 +33,7 @@ public:
     ~ParallelReadBuffer() override { finishAndWait(); }
 
     off_t seek(off_t off, int whence) override;
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
     off_t getPosition() override;
 
     const SeekableReadBuffer & getReadBuffer() const { return input; }

src/IO/ReadBufferFromEmptyFile.h

@@ -19,7 +19,8 @@ private:
     std::string getFileName() const override { return "<empty>"; }
     off_t seek(off_t /*off*/, int /*whence*/) override { return 0; }
     off_t getPosition() override { return 0; }
-    size_t getFileSize() override { return 0; }
+    std::optional<size_t> tryGetFileSize() override { return 0; }
+    size_t getFileOffsetOfBufferEnd() const override { return 0; }
 };
 
 }

src/IO/ReadBufferFromEncryptedFile.h

@@ -30,7 +30,7 @@ public:
 
     void setReadUntilEnd() override { in->setReadUntilEnd(); }
 
-    size_t getFileSize() override { return in->getFileSize(); }
+    std::optional<size_t> tryGetFileSize() override { return in->tryGetFileSize(); }
 
 private:
     bool nextImpl() override;

src/IO/ReadBufferFromFileBase.cpp

@@ -5,11 +5,6 @@
 namespace DB
 {
 
-namespace ErrorCodes
-{
-    extern const int UNKNOWN_FILE_SIZE;
-}
-
 ReadBufferFromFileBase::ReadBufferFromFileBase() : BufferWithOwnMemory<SeekableReadBuffer>(0)
 {
 }
@@ -26,11 +21,9 @@ ReadBufferFromFileBase::ReadBufferFromFileBase(
 
 ReadBufferFromFileBase::~ReadBufferFromFileBase() = default;
 
-size_t ReadBufferFromFileBase::getFileSize()
+std::optional<size_t> ReadBufferFromFileBase::tryGetFileSize()
 {
-    if (file_size)
-        return *file_size;
-    throw Exception(ErrorCodes::UNKNOWN_FILE_SIZE, "Cannot find out file size for read buffer");
+    return file_size;
 }
 
 void ReadBufferFromFileBase::setProgressCallback(ContextPtr context)

src/IO/ReadBufferFromFileBase.h

@@ -50,7 +50,7 @@ public:
         clock_type = clock_type_;
     }
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     void setProgressCallback(ContextPtr context);
 

src/IO/ReadBufferFromFileDecorator.cpp

@@ -52,9 +52,9 @@ bool ReadBufferFromFileDecorator::nextImpl()
     return result;
 }
 
-size_t ReadBufferFromFileDecorator::getFileSize()
+std::optional<size_t> ReadBufferFromFileDecorator::tryGetFileSize()
 {
-    return getFileSizeFromReadBuffer(*impl);
+    return tryGetFileSizeFromReadBuffer(*impl);
 }
 
 }

src/IO/ReadBufferFromFileDecorator.h

@@ -27,7 +27,7 @@ public:
 
     ReadBuffer & getWrappedReadBuffer() { return *impl; }
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
 protected:
     std::unique_ptr<SeekableReadBuffer> impl;

src/IO/ReadBufferFromFileDescriptor.cpp

@@ -253,7 +253,7 @@ void ReadBufferFromFileDescriptor::rewind()
     file_offset_of_buffer_end = 0;
 }
 
-size_t ReadBufferFromFileDescriptor::getFileSize()
+std::optional<size_t> ReadBufferFromFileDescriptor::tryGetFileSize()
 {
     return getSizeFromFileDescriptor(fd, getFileName());
 }

src/IO/ReadBufferFromFileDescriptor.h

@@ -69,7 +69,7 @@ public:
     /// Seek to the beginning, discarding already read data if any. Useful to reread file that changes on every read.
     void rewind();
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     bool checkIfActuallySeekable() override;
 

src/IO/ReadBufferFromS3.cpp

@@ -311,15 +311,15 @@ off_t ReadBufferFromS3::seek(off_t offset_, int whence)
     return offset;
 }
 
-size_t ReadBufferFromS3::getFileSize()
+std::optional<size_t> ReadBufferFromS3::tryGetFileSize()
 {
     if (file_size)
-        return *file_size;
+        return file_size;
 
     auto object_size = S3::getObjectSize(*client_ptr, bucket, key, version_id);
 
     file_size = object_size;
-    return *file_size;
+    return file_size;
 }
 
 off_t ReadBufferFromS3::getPosition()

src/IO/ReadBufferFromS3.h

@@ -63,7 +63,7 @@ public:
 
     off_t getPosition() override;
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     void setReadUntilPosition(size_t position) override;
     void setReadUntilEnd() override;

src/IO/ReadWriteBufferFromHTTP.cpp

@@ -72,7 +72,6 @@ namespace ErrorCodes
     extern const int BAD_ARGUMENTS;
     extern const int CANNOT_SEEK_THROUGH_FILE;
     extern const int SEEK_POSITION_OUT_OF_BOUND;
-    extern const int UNKNOWN_FILE_SIZE;
 }
 
 std::unique_ptr<ReadBuffer> ReadWriteBufferFromHTTP::CallResult::transformToReadBuffer(size_t buf_size) &&
@@ -121,15 +120,33 @@ void ReadWriteBufferFromHTTP::prepareRequest(Poco::Net::HTTPRequest & request, s
         credentials.authenticate(request);
 }
 
-size_t ReadWriteBufferFromHTTP::getFileSize()
+std::optional<size_t> ReadWriteBufferFromHTTP::tryGetFileSize()
 {
     if (!file_info)
-        file_info = getFileInfo();
+    {
+        try
+        {
+            file_info = getFileInfo();
+        }
+        catch (const HTTPException &)
+        {
+            return std::nullopt;
+        }
+        catch (const NetException &)
+        {
+            return std::nullopt;
+        }
+        catch (const Poco::Net::NetException &)
+        {
+            return std::nullopt;
+        }
+        catch (const Poco::IOException &)
+        {
+            return std::nullopt;
+        }
+    }
 
-    if (file_info->file_size)
-        return *file_info->file_size;
-
-    throw Exception(ErrorCodes::UNKNOWN_FILE_SIZE, "Cannot find out file size for: {}", initial_uri.toString());
+    return file_info->file_size;
 }
 
 bool ReadWriteBufferFromHTTP::supportsReadAt()
@@ -311,12 +328,12 @@ void ReadWriteBufferFromHTTP::doWithRetries(std::function<void()> && callable,
             error_message = e.displayText();
             exception = std::current_exception();
         }
-        catch (DB::NetException & e)
+        catch (NetException & e)
         {
             error_message = e.displayText();
             exception = std::current_exception();
         }
-        catch (DB::HTTPException & e)
+        catch (HTTPException & e)
         {
             if (!isRetriableError(e.getHTTPStatus()))
                 is_retriable = false;
@@ -324,7 +341,7 @@ void ReadWriteBufferFromHTTP::doWithRetries(std::function<void()> && callable,
             error_message = e.displayText();
             exception = std::current_exception();
         }
-        catch (DB::Exception & e)
+        catch (Exception & e)
         {
             is_retriable = false;
 
@@ -683,7 +700,19 @@ std::optional<time_t> ReadWriteBufferFromHTTP::tryGetLastModificationTime()
         {
             file_info = getFileInfo();
         }
-        catch (...)
+        catch (const HTTPException &)
+        {
+            return std::nullopt;
+        }
+        catch (const NetException &)
+        {
+            return std::nullopt;
+        }
+        catch (const Poco::Net::NetException &)
+        {
+            return std::nullopt;
+        }
+        catch (const Poco::IOException &)
         {
             return std::nullopt;
         }
@@ -704,7 +733,7 @@ ReadWriteBufferFromHTTP::HTTPFileInfo ReadWriteBufferFromHTTP::getFileInfo()
     {
         getHeadResponse(response);
     }
-    catch (HTTPException & e)
+    catch (const HTTPException & e)
     {
         /// Maybe the web server doesn't support HEAD requests.
         /// E.g. webhdfs reports status 400.

src/IO/ReadWriteBufferFromHTTP.h

@@ -118,7 +118,7 @@ private:
 
     std::unique_ptr<ReadBuffer> initialize();
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     bool supportsReadAt() override;
 

src/IO/WithFileSize.cpp

@@ -13,41 +13,47 @@ namespace ErrorCodes
     extern const int UNKNOWN_FILE_SIZE;
 }
 
-template <typename T>
-static size_t getFileSize(T & in)
+size_t WithFileSize::getFileSize()
 {
-    if (auto * with_file_size = dynamic_cast<WithFileSize *>(&in))
-    {
-        return with_file_size->getFileSize();
-    }
+    if (auto maybe_size = tryGetFileSize())
+        return *maybe_size;
 
     throw Exception(ErrorCodes::UNKNOWN_FILE_SIZE, "Cannot find out file size");
 }
 
-size_t getFileSizeFromReadBuffer(ReadBuffer & in)
+template <typename T>
+static std::optional<size_t> tryGetFileSize(T & in)
 {
-    if (auto * delegate = dynamic_cast<ReadBufferFromFileDecorator *>(&in))
-    {
-        return getFileSize(delegate->getWrappedReadBuffer());
-    }
-    else if (auto * compressed = dynamic_cast<CompressedReadBufferWrapper *>(&in))
-    {
-        return getFileSize(compressed->getWrappedReadBuffer());
-    }
+    if (auto * with_file_size = dynamic_cast<WithFileSize *>(&in))
+        return with_file_size->tryGetFileSize();
 
-    return getFileSize(in);
+    return std::nullopt;
+}
+
+template <typename T>
+static size_t getFileSize(T & in)
+{
+    if (auto maybe_size = tryGetFileSize(in))
+        return *maybe_size;
+
+    throw Exception(ErrorCodes::UNKNOWN_FILE_SIZE, "Cannot find out file size");
 }
 
 std::optional<size_t> tryGetFileSizeFromReadBuffer(ReadBuffer & in)
 {
-    try
-    {
-        return getFileSizeFromReadBuffer(in);
-    }
-    catch (...)
-    {
-        return std::nullopt;
-    }
+    if (auto * delegate = dynamic_cast<ReadBufferFromFileDecorator *>(&in))
+        return tryGetFileSize(delegate->getWrappedReadBuffer());
+    else if (auto * compressed = dynamic_cast<CompressedReadBufferWrapper *>(&in))
+        return tryGetFileSize(compressed->getWrappedReadBuffer());
+    return tryGetFileSize(in);
+}
+
+size_t getFileSizeFromReadBuffer(ReadBuffer & in)
+{
+    if (auto maybe_size = tryGetFileSizeFromReadBuffer(in))
+        return *maybe_size;
+
+    throw Exception(ErrorCodes::UNKNOWN_FILE_SIZE, "Cannot find out file size");
 }
 
 bool isBufferWithFileSize(const ReadBuffer & in)

src/IO/WithFileSize.h

@@ -10,15 +10,16 @@ class ReadBuffer;
 class WithFileSize
 {
 public:
-    virtual size_t getFileSize() = 0;
+    /// Returns nullopt if couldn't find out file size;
+    virtual std::optional<size_t> tryGetFileSize() = 0;
     virtual ~WithFileSize() = default;
+
+    size_t getFileSize();
 };
 
 bool isBufferWithFileSize(const ReadBuffer & in);
 
 size_t getFileSizeFromReadBuffer(ReadBuffer & in);
-
-/// Return nullopt if couldn't find out file size;
 std::optional<size_t> tryGetFileSizeFromReadBuffer(ReadBuffer & in);
 
 size_t getDataOffsetMaybeCompressed(const ReadBuffer & in);

src/Storages/Cache/ExternalDataSourceCache.h

@@ -53,7 +53,7 @@ public:
     bool nextImpl() override;
     off_t seek(off_t off, int whence) override;
     off_t getPosition() override;
-    size_t getFileSize() override { return remote_file_size; }
+    std::optional<size_t> tryGetFileSize() override { return remote_file_size; }
 
 private:
     std::unique_ptr<LocalFileHolder> local_file_holder;

src/Storages/ObjectStorage/HDFS/AsynchronousReadBufferFromHDFS.cpp

@@ -91,9 +91,9 @@ void AsynchronousReadBufferFromHDFS::prefetch(Priority priority)
 }
 
 
-size_t AsynchronousReadBufferFromHDFS::getFileSize()
+std::optional<size_t> AsynchronousReadBufferFromHDFS::tryGetFileSize()
 {
-    return impl->getFileSize();
+    return impl->tryGetFileSize();
 }
 
 String AsynchronousReadBufferFromHDFS::getFileName() const

src/Storages/ObjectStorage/HDFS/AsynchronousReadBufferFromHDFS.h

@@ -35,7 +35,7 @@ public:
 
     void prefetch(Priority priority) override;
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     String getFileName() const override;
 

src/Storages/ObjectStorage/HDFS/ReadBufferFromHDFS.cpp

@@ -31,7 +31,7 @@ namespace ErrorCodes
 }
 
 
-struct ReadBufferFromHDFS::ReadBufferFromHDFSImpl : public BufferWithOwnMemory<SeekableReadBuffer>
+struct ReadBufferFromHDFS::ReadBufferFromHDFSImpl : public BufferWithOwnMemory<SeekableReadBuffer>, public WithFileSize
 {
     String hdfs_uri;
     String hdfs_file_path;
@@ -90,7 +90,7 @@ struct ReadBufferFromHDFS::ReadBufferFromHDFSImpl : public BufferWithOwnMemory<S
         hdfsCloseFile(fs.get(), fin);
     }
 
-    size_t getFileSize() const
+    std::optional<size_t> tryGetFileSize() override
     {
         return file_size;
     }
@@ -191,9 +191,9 @@ ReadBufferFromHDFS::ReadBufferFromHDFS(
 
 ReadBufferFromHDFS::~ReadBufferFromHDFS() = default;
 
-size_t ReadBufferFromHDFS::getFileSize()
+std::optional<size_t> ReadBufferFromHDFS::tryGetFileSize()
 {
-    return impl->getFileSize();
+    return impl->tryGetFileSize();
 }
 
 bool ReadBufferFromHDFS::nextImpl()

src/Storages/ObjectStorage/HDFS/ReadBufferFromHDFS.h

@@ -40,7 +40,7 @@ public:
 
     off_t getPosition() override;
 
-    size_t getFileSize() override;
+    std::optional<size_t> tryGetFileSize() override;
 
     size_t getFileOffsetOfBufferEnd() const override;
 

tests/queries/0_stateless/03173_row_binary_and_native_with_binary_encoded_types.sh

@@ -6,8 +6,8 @@ CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
 
 function test
 {
-  $CLICKHOUSE_LOCAL --allow_experimental_dynamic_type=1 --allow_experimental_variant_type=1 --output_format_binary_encode_types_in_binary_format=1 -q "select $1 as value format RowBinaryWithNamesAndTypes" | $CLICKHOUSE_LOCAL --input-format RowBinaryWithNamesAndTypes --input_format_binary_decode_types_in_binary_format=1 -q "select value, toTypeName(value) from table"
-  $CLICKHOUSE_LOCAL --allow_experimental_dynamic_type=1 --allow_experimental_variant_type=1 --output_format_native_encode_types_in_binary_format=1 -q "select $1 as value format Native" | $CLICKHOUSE_LOCAL --input-format Native --input_format_native_decode_types_in_binary_format=1 -q "select value, toTypeName(value) from table"
+    $CLICKHOUSE_LOCAL --allow_experimental_dynamic_type=1 --allow_experimental_variant_type=1 --output_format_binary_encode_types_in_binary_format=1 -q "select $1 as value format RowBinaryWithNamesAndTypes" | $CLICKHOUSE_LOCAL --input-format RowBinaryWithNamesAndTypes --input_format_binary_decode_types_in_binary_format=1 -q "select value, toTypeName(value) from table"
+    $CLICKHOUSE_LOCAL --allow_experimental_dynamic_type=1 --allow_experimental_variant_type=1 --output_format_native_encode_types_in_binary_format=1 -q "select $1 as value format Native" | $CLICKHOUSE_LOCAL --input-format Native --input_format_native_decode_types_in_binary_format=1 -q "select value, toTypeName(value) from table"
 }
 
 test "materialize(42)::UInt8"

tests/queries/0_stateless/03206_no_exceptions_clickhouse_local.reference

@@ -0,0 +1 @@
+Hello	world

tests/queries/0_stateless/03206_no_exceptions_clickhouse_local.sh

@@ -0,0 +1,7 @@
+#!/usr/bin/env bash
+
+CURDIR=$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)
+# shellcheck source=../shell_config.sh
+. "$CURDIR"/../shell_config.sh
+
+CLICKHOUSE_TERMINATE_ON_ANY_EXCEPTION=1 ${CLICKHOUSE_LOCAL} --query "SELECT * FROM table" --input-format CSV <<<"Hello, world"