Date: Mon, 6 Apr 2020 20:49:25 -0400
From: Alvaro Herrera <alvherre@2ndquadrant.com>
To: Surafel Temesgen <surafel3000@gmail.com>
Cc: Tomas Vondra <tomas.vondra@2ndquadrant.com>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
	Andrew Gierth <andrew@tao11.riddles.org.uk>
Subject: Re: FETCH FIRST clause WITH TIES option
Message-ID: <20200407004925.GA23627@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20200407003502.GA20914@alvherre.pgsql>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

Some ruleutils.c code added by this patch is not covered by tests:

    5246             :     /* Add the LIMIT clause if given */
    5247        1115 :     if (query->limitOffset != NULL)
    5248             :     {
    5249           0 :         appendContextKeyword(context, " OFFSET ",
    5250             :                              -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
    5251           0 :         get_rule_expr(query->limitOffset, context, false);
    5252             :     }
    5253        1115 :     if (query->limitOption == LIMIT_OPTION_WITH_TIES)
    5254             :     {
    5255           0 :         appendContextKeyword(context, " FETCH FIRST ",
    5256             :                              -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
    5257           0 :         get_rule_expr(query->limitCount, context, false);
    5258           0 :         appendContextKeyword(context, " ROWS WITH TIES ",
    5259             :                              -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
    5260             :     }
    5261        1115 :     if (query->limitCount != NULL && query->limitOption != LIMIT_OPTION_WITH_TIES)
    5262             :     {
    5263           2 :         appendContextKeyword(context, " LIMIT ",
    5264             :                              -PRETTYINDENT_STD, PRETTYINDENT_STD, 0);
    5265           2 :         if (IsA(query->limitCount, Const) &&
    5266           0 :             ((Const *) query->limitCount)->constisnull)
    5267           0 :             appendStringInfoString(buf, "ALL");
    5268             :         else
    5269           2 :             get_rule_expr(query->limitCount, context, false);
    5270             :     }

Other than that, the patch seems good to go to me, so unless there are
objections, I intend to get this pushed tomorrow.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services