public inbox for [email protected]  

help / color / mirror / Atom feed

From: Alvaro Herrera <[email protected]>
To: Tom Lane <[email protected]>
Cc: Zhang Mingli <[email protected]>
Cc: Surafel Temesgen <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: useless LIMIT_OPTION_DEFAULT
Date: Sat, 16 Dec 2023 17:42:28 +0100
Message-ID: <[email protected]> (raw)
In-Reply-To: <[email protected]>

On 2023-Dec-14, Tom Lane wrote:

> Zhang Mingli <[email protected]> writes:
> > By reading the codes, I found that we process limit option as LIMIT_OPTION_WITH_TIES when using WITH TIES
> > and all others are LIMIT_OPTION_COUNT by  commit 357889eb17bb9c9336c4f324ceb1651da616fe57.
> > And check actual limit node in limit_needed().
> > There is no need to maintain a useless default limit enum.
> 
> I agree, that looks pretty useless.  Our normal convention for
> representing not having any LIMIT clause would be to not create
> a SelectLimit node at all.  I don't see why allowing a second
> representation is a good idea, especially when the code is not
> in fact doing that.
> 
> git blame shows that this came in with 357889eb1.  Alvaro,
> Surafel, do you want to argue for keeping things as-is?

I looked at the history of this.  That enum member first appeared as a
result of your review at [1], and the accompanying code at that time did
use it a few times.  The problem is that I later ([2]) proposed to rewrite
that code and remove most of the uses of that enum member, but failed to
remove it completely.

So, I think we're OK to remove it.  I'm going to push Zhang's patch
soon unless there are objections.

[1] https://postgr.es/m/[email protected]
[2] https://postgr.es/m/[email protected]

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
"Doing what he did amounts to sticking his fingers under the hood of the
implementation; if he gets his fingers burnt, it's his problem."  (Tom Lane)

view thread (3+ messages)

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: useless LIMIT_OPTION_DEFAULT
  In-Reply-To: <[email protected]>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox