Re: [PATCH v1 14/18] Add delete_broadcast() in scraper/db.py

[Ammar Faizi <ammarfaizi2@gnuweeb.org>]

On 7/27/22 9:46 AM, Muhammad Rizki wrote:
> +	def delete_broadcast(self, chat_id: Union[int, str]):
> +		get_query = """
> +			SELECT id FROM broadcast_chats
> +			WHERE chat_id = %(chat_id)s
> +		"""
> +		delete_query = """
> +			DELETE FROM broadcast_chats
> +			WHERE chat_id = %(chat_id)s
> +		"""
> +		self.cur.execute(get_query, {"chat_id": chat_id})
> +		exist = self.cur.fetchone()
> +		if bool(exist):
> +			self.cur.execute(delete_query, {"chat_id": chat_id})
> +			return True
> +
> +		return False

What?! What is the purpose of the SELECT query here?

    - You check whether there is at least a row that matches with the chat_id.
    - If there exists, you delete all rows that match with the chat_id.

Why not just run the DELETE query directly? The SELECT query here is useless!

You may be wondering: "How can I return False if there is no matching row?".
Simple: Check the affected rows of the DELETE query.

IOW, something like this:

	def delete_broadcast(self, chat_id: Union[int, str]):
		delete_query = """
			DELETE FROM broadcast_chats WHERE chat_id = %(chat_id)s
		"""
		self.cur.execute(delete_query, {"chat_id": chat_id})
		return self.cur.rowcount > 0

Totally untested, but you get the idea. No?

-- 
Ammar Faizi