public inbox for [email protected]
help / color / mirror / Atom feedFrom: Dmitry Dolgov <[email protected]>
To: Zhihong Yu <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Greg Stark <[email protected]>
Cc: Pavel Trukhanov <[email protected]>
Subject: Re: pg_stat_statements and "IN" conditions
Date: Tue, 5 Jan 2021 13:52:30 +0100
Message-ID: <20210105125230.etgmgixnn6fmpszl@localhost> (raw)
In-Reply-To: <CALNJ-vQx7u-Nq_wVun72CVrHe4DcGfEurj_ikckJrRy2Pem=JA@mail.gmail.com>
References: <CA+q6zcWtUbT_Sxj0V6HY6EZ89uv5wuG5aefpe_9n0Jr3VwntFg@mail.gmail.com>
<20201118160432.hyko73sqc2bf7nkj@localhost>
<20201226104635.uvzkftzu72zg5qux@localhost>
<CALNJ-vQx7u-Nq_wVun72CVrHe4DcGfEurj_ikckJrRy2Pem=JA@mail.gmail.com>
> On Sat, Dec 26, 2020 at 08:53:28AM -0800, Zhihong Yu wrote:
> Hi,
> A few comments.
>
> + foreach(lc, (List *) expr)
> + {
> + Node * subExpr = (Node *) lfirst(lc);
> +
> + if (!IsA(subExpr, Const))
> + {
> + allConst = false;
> + break;
> + }
> + }
>
> It seems the above foreach loop (within foreach(temp, (List *) node)) can
> be preceded with a check that allConst is true. Otherwise the loop can be
> skipped.
Thanks for noticing. Now that I look at it closer I think it's the other
way around, the loop above checking constants for the first expression
is not really necessary.
> + if (currentExprIdx == pgss_merge_threshold - 1)
> + {
> + JumbleExpr(jstate, expr);
> +
> + /*
> + * A const expr is already found, so JumbleExpr must
> + * record it. Mark it as merged, it will be the
> first
> + * merged but still present in the statement query.
> + */
> + Assert(jstate->clocations_count > 0);
> + jstate->clocations[jstate->clocations_count -
> 1].merged = true;
> + currentExprIdx++;
> + }
>
> The above snippet occurs a few times. Maybe extract into a helper method.
Originally I was hesitant to extract it was because it's quite small
part of the code. But now I've realized that the part relevant to lists
is not really correct, which makes those bits even more different, so I
think it makes sense to leave it like that. What do you think?
view thread (38+ messages) latest in thread
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: pg_stat_statements and "IN" conditions
In-Reply-To: <20210105125230.etgmgixnn6fmpszl@localhost>
* 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