Add requested PR changes. [#CLICKHOUSE-2145] [issue #325]

dbms/include/DB/IO/WriteBufferFromHTTPServerResponse.h

@@ -42,7 +42,6 @@ private:
 	Poco::Net::HTTPServerResponse & response;
 
 	bool add_cors_header;
-	bool add_authenticate_header = false;
 	bool compress;
 	ZlibCompressionMethod compression_method;
 	int compression_level = Z_DEFAULT_COMPRESSION;
@@ -77,11 +76,6 @@ private:
 			if (add_cors_header)
 				response.set("Access-Control-Allow-Origin", "*");
 
-			if (add_authenticate_header)
-			{
-				response.set("WWW-Authenticate", "Basic realm=\"User Visible Realm\"");
-			}
-
 			setResponseDefaultHeaders(response);
 
 			std::tie(response_header_ostr, response_body_ostr) = response.beginSend();
@@ -221,11 +215,6 @@ public:
 		send_progress_interval_ms = send_progress_interval_ms_;
 	}
 
-	void addAuthenticateRequest()
-	{
-		add_authenticate_header = true;
-	}
-
 	~WriteBufferFromHTTPServerResponse()
 	{
 		try

dbms/include/DB/Interpreters/Context.h

@@ -284,6 +284,7 @@ public:
 	QueryLog & getQueryLog();
 	const MergeTreeSettings & getMergeTreeSettings();
 
+	/// Prevents DROP TABLE if its size is greater than max_size (50GB by default, max_size=0 turn off this check)
 	void setMaxTableSizeToDrop(size_t max_size);
 	void checkTableCanBeDropped(const String & database, const String & table, size_t table_size);
 

dbms/include/DB/Storages/IStorage.h

@@ -271,12 +271,17 @@ public:
 
 	bool is_dropped{false};
 
-	/// Поддерживается ли индекс в секции IN
+	/// Does table support index for IN sections
 	virtual bool supportsIndexForIn() const { return false; }
 
-	/// проверяет валидность данных
+	/// Checks validity of the data
 	virtual bool checkData() const { throw DB::Exception("Check query is not supported for " + getName() + " storage"); }
 
+	/// Checks that table could be dropped right now
+	/// If it can - returns true
+	/// Otherwise - throws an exception with detailed information or returns false
+	virtual bool checkTableCanBeDropped() const { return true; }
+
 protected:
 	using ITableDeclaration::ITableDeclaration;
 	using std::enable_shared_from_this<IStorage>::shared_from_this;

dbms/include/DB/Storages/StorageMergeTree.h

@@ -103,7 +103,10 @@ public:
 
 	bool supportsIndexForIn() const override { return true; }
 
+	bool checkTableCanBeDropped() const override;
+
 	MergeTreeData & getData() { return data; }
+	const MergeTreeData & getData() const { return data; }
 
 private:
 	String path;

dbms/include/DB/Storages/StorageReplicatedMergeTree.h

@@ -156,7 +156,10 @@ public:
 
 	bool supportsIndexForIn() const override { return true; }
 
+	bool checkTableCanBeDropped() const override;
+
 	MergeTreeData & getData() { return data; }
+	const MergeTreeData & getData() const { return data; }
 	MergeTreeData * getUnreplicatedData() { return unreplicated_data.get(); }
 
 

dbms/src/Core/ErrorCodes.cpp

@@ -361,6 +361,7 @@ namespace ErrorCodes
 	extern const int BAD_LAMBDA = 356;
 	extern const int RESERVED_IDENTIFIER_NAME = 357;
 	extern const int INTO_OUTFILE_NOT_ALLOWED = 358;
+	extern const int TABLE_SIZE_EXCEED_MAX_DROP_SIZE_LIMIT = 359;
 
 	extern const int KEEPER_EXCEPTION = 999;
 	extern const int POCO_EXCEPTION = 1000;

dbms/src/Interpreters/Context.cpp

@@ -72,6 +72,7 @@ namespace ErrorCodes
 	extern const int THERE_IS_NO_SESSION;
 	extern const int NO_ELEMENTS_IN_CONFIG;
 	extern const int DDL_GUARD_IS_ACTIVE;
+	extern const int TABLE_SIZE_EXCEED_MAX_DROP_SIZE_LIMIT;
 }
 
 class TableFunctionFactory;
@@ -1114,28 +1115,32 @@ void Context::checkTableCanBeDropped(const String & database, const String & tab
 	{
 		try
 		{
-			force_file.remove(true);
+			force_file.remove();
 			return;
 		}
 		catch (...)
 		{
+			/// User should recreate force file on each drop, it shouldn't be protected
+			tryLogCurrentException("Drop table check", "Can't remove force file to enable table drop");
 		}
 	}
 
 	String table_size_str = formatReadableSizeWithDecimalSuffix(table_size);
 	String max_table_size_to_drop_str = formatReadableSizeWithDecimalSuffix(max_table_size_to_drop);
+	std::stringstream ostr;
 
-	throw Exception(
-		"Table " + database + "." + table + " was not dropped.\n"
-		"Reason:\n"
-		"1) Table size (" + table_size_str + ") is greater than max_table_size_to_drop (" + max_table_size_to_drop_str + ")\n"
-		"2) File " + force_file.path() + " intedned to force DROP " +
-			(force_file_exists ? String("exists but not writeable (could not be removed)") : String("doesn't exists")) + "\n"
-		"How to fix this:\n"
-		"1) Either increase (or set to zero) max_table_size_to_drop in server config and restart clickhouse\n"
-		"2) Either create forcing file " + force_file.path() + " and make sure that clickhouse has written permission for it.\n"
-		"2) bash example: touch '" + force_file.path() + "' && chmod 0777 '" + force_file.path() + "'"
-		, ErrorCodes::TABLE_WAS_NOT_DROPPED);
+	ostr << "Table " << database << "." << table << " was not dropped.\n"
+		 << "Reason:\n"
+		 << "1. Table size (" << table_size_str << ") is greater than max_table_size_to_drop (" << max_table_size_to_drop_str << ")\n"
+		 << "2. File " << force_file.path() << " intedned to force DROP "
+			<< (force_file_exists ? "exists but not writeable (could not be removed)" : "doesn't exist") << "\n";
+
+	ostr << "How to fix this:\n"
+		 << "1. Either increase (or set to zero) max_table_size_to_drop in server config and restart clickhouse\n"
+		 << "2. Either create forcing file " << force_file.path() << " and make sure that clickhouse has written permission for it.\n"
+		 << "2. bash example: touch '" << force_file.path() << "' && chmod 0777 '" << force_file.path() << "'";
+
+	throw Exception(ostr.str(), ErrorCodes::TABLE_SIZE_EXCEED_MAX_DROP_SIZE_LIMIT);
 }
 
 

dbms/src/Interpreters/InterpreterDropQuery.cpp

@@ -86,22 +86,26 @@ BlockIO InterpreterDropQuery::execute()
 	{
 		table.first->shutdown();
 
-		/// Если кто-то успел удалить эту таблицу, выбросит исключение.
+		/// If table was already dropped by anyone, an exception will be thrown
 		auto table_lock = table.first->lockForAlter();
 
 		String current_table_name = table.first->getTableName();
 
 		if (drop.detach)
 		{
-			/// Удаляем таблицу из оперативки, метаданные и данные не трогаем.
+			/// Drop table from memory, don't touch data and metadata
 			database->detachTable(current_table_name);
 		}
 		else
 		{
-			/// Delete table data
-			table.first->drop();
+			if (!table.first->checkTableCanBeDropped())
+				throw Exception("Table " + database_name  + "." + current_table_name + " couldn't be dropped due to failed pre-drop check",
+								ErrorCodes::TABLE_WAS_NOT_DROPPED);
+
 			/// Delete table metdata and table itself from memory
 			database->removeTable(current_table_name);
+			/// Delete table data
+			table.first->drop();
 
 			table.first->is_dropped = true;
 

dbms/src/Server/HTTPHandler.cpp

@@ -294,8 +294,7 @@ void HTTPHandler::trySendExceptionToClient(const std::string & s, int exception_
 
 		if (auth_fail)
 		{
-			response.setStatusAndReason(Poco::Net::HTTPResponse::HTTP_UNAUTHORIZED);
-			used_output.out->addAuthenticateRequest();
+			response.requireAuthentication("ClickHouse server HTTP API");
 		}
 		else
 		{

dbms/src/Storages/StorageMergeTree.cpp

@@ -124,10 +124,14 @@ BlockOutputStreamPtr StorageMergeTree::write(ASTPtr query, const Settings & sett
 	return std::make_shared<MergeTreeBlockOutputStream>(*this);
 }
 
-void StorageMergeTree::drop()
+bool StorageMergeTree::checkTableCanBeDropped() const
 {
 	context.checkTableCanBeDropped(database_name, table_name, getData().getColumnsTotalSize());
+	return true;
+}
 
+void StorageMergeTree::drop()
+{
 	shutdown();
 	data.dropAllData();
 }

dbms/src/Storages/StorageReplicatedMergeTree.cpp

@@ -2803,6 +2803,12 @@ void StorageReplicatedMergeTree::attachPartition(ASTPtr query, const Field & fie
 	}
 }
 
+bool StorageReplicatedMergeTree::checkTableCanBeDropped() const
+{
+	/// Consider only synchronized data
+	context.checkTableCanBeDropped(database_name, table_name, getData().getColumnsTotalSize());
+	return true;
+}
 
 void StorageReplicatedMergeTree::drop()
 {
@@ -2812,8 +2818,7 @@ void StorageReplicatedMergeTree::drop()
 		if (is_readonly || !zookeeper)
 			throw Exception("Can't drop readonly replicated table (need to drop data in ZooKeeper as well)", ErrorCodes::TABLE_IS_READ_ONLY);
 
-		/// Consider only synchronized data
-		context.checkTableCanBeDropped(database_name, table_name, getData().getColumnsTotalSize());
+		// checkTableCanBeDropped(); // uncomment to feel yourself safe
 
 		shutdown();