Content-Type: text/plain;
	charset=utf-8
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3826.700.81\))
Subject: Re: generic plans and "initial" pruning
From: Chao Li <li.evan.chao@gmail.com>
In-Reply-To: 
 <CA+HiwqGJP91Qed0EjuB72Lv4_QAiVOMYjya7GA0aas8K6NZUZA@mail.gmail.com>
Date: Mon, 24 Nov 2025 11:29:53 +0800
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
 Tender Wang <tndrwang@gmail.com>,
 Alexander Lakhin <exclusion@gmail.com>,
 Tomas Vondra <tomas@vondra.me>,
 Robert Haas <robertmhaas@gmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
 Andres Freund <andres@anarazel.de>,
 Daniel Gustafsson <daniel@yesql.se>,
 David Rowley <dgrowleyml@gmail.com>,
 PostgreSQL Hackers <pgsql-hackers@postgresql.org>,
 Thom Brown <thom@linux.com>
Content-Transfer-Encoding: quoted-printable
Message-Id: <A93C2AC9-8B93-45AB-B56C-E6771D988808@gmail.com>
References: 
 <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
 <54c35fb9-da3a-4754-ab8c-46ed0b612465@vondra.me>
 <CA+HiwqH8N-SxEB6SysEBsYNgV_KJs66k9Z2SNmqVzbBP-60yWg@mail.gmail.com>
 <684c70d7-180e-461d-9377-600c2db581ba@vondra.me>
 <CA+HiwqEmG9YCQvG6uux7sO=jKFSAW6hA4Ea-ymfD+JhJAe4PWQ@mail.gmail.com>
 <CA+HiwqE2FfJfH=siLiR3kJ13tmXZORAGTWsZc2r52o1_5BDv+g@mail.gmail.com>
 <bfcc1b78-aa39-4356-8f42-b8cfb9733d6e@vondra.me>
 <CA+HiwqFhkpXHAA=4NY5SqYXX08uq=nYtXcSByNZF=2MAy1UA7A@mail.gmail.com>
 <CA+HiwqHCcSoYfpMjFshaU1bj6NjreiDvMSDpVSeBmqk-kbWrPw@mail.gmail.com>
 <CA+HiwqHOejJk0_qMuM5g38h70hY_JvHMAKwnH3k=urfTXauPQA@mail.gmail.com>
 <CA+HiwqFsGKM82oaMby3VWYXf_XFpDAMeT+6SXgj-45HpTrS1dA@mail.gmail.com>
 <CA+HiwqFA5hUWYktt3VMh4zQOYMxqH-MpdX8eemfM+o-9dY-zbQ@mail.gmail.com>
 <CA+HiwqEn7bbUXaXO=SmUujBjJSHfS31cwQroHRBwT0sR=66bgg@mail.gmail.com>
 <CA+HiwqGGLDTd1ZTK1c0zv4La7XOVSVMqBuNtscJeh6FyUQvFvA@mail.gmail.com>
 <CA+HiwqE2JFiqqrXdyJVQWY-fMGwzDkLqjXQdUKbPaCpDpxd_2g@mail.gmail.com>
 <CA+HiwqFp3jZGSz==QjeuV62_62F6+V6b62=Uqvy99sW_gsgWBA@mail.gmail.com>
 <CAHewXNkUz9XGG8nnoxZaw35e+5bQVVP=eeJE4cW4V2e+P9ndFw@mail.gmail.com>
 <CA+HiwqFKSpfYruzcVz-5CcFxg7gMa+ycXjMa2aPz_J_P4LGXTg@mail.gmail.com>
 <605328.1747710381@sss.pgh.pa.us>
 <CA+HiwqEQ1oME-hcDXwC9rGQb=u7MdUFG3Sc=Qg27uH480v10FA@mail.gmail.com>
 <691261.1747755511@sss.pgh.pa.us>
 <CA+HiwqGXMLSQyJvynWF40yNwBAx-pXtxemReP8L+C+kaUa5v5A@mail.gmail.com>
 <CA+HiwqGBfMgcxokEH_mg6s=ttLFm54dj4hT6yXydU2t0g6oQ3g@mail.gmail.com>
 <CA+HiwqEEkGfMc_LSJhfz96o-czVS4B59Vhw6i1_t58ZGqhP8VA@mail.gmail.com>
 <CA+HiwqHAd+9nptjxP6=KrcKA1BMsS6pbB3B2oaojwdyH_wBWCA@mail.gmail.com>
 <CA+HiwqE7_YpU--EsrhvNqcZ+10+92EGFaX5609AUJb9ENLntnQ@mail.gmail.com>
 <CA+HiwqEF9SgKyQ1HrYOURpv8DGRGHDNwBT9Y6yEBVCW+=kh_=w@mail.gmail.com>
 <CA+HiwqFpEHBjosRackQhm6yKKnHgqm8Ewkn=qsctT1N0PqVSrg@mail.gmail.com>
 <CA+HiwqGJP91Qed0EjuB72Lv4_QAiVOMYjya7GA0aas8K6NZUZA@mail.gmail.com>
To: Amit Langote <amitlangote09@gmail.com>
Archived-At: 
 <https://www.postgresql.org/message-id/A93C2AC9-8B93-45AB-B56C-E6771D988808%40gmail.com>
Precedence: bulk

Hi, Amit,

Locking only surviving partitions sounds a good optimization. I started =
to review this patch, but I cannot finish reviewing in one day. I will =
post my comments as long as I finished some commits.

> On Nov 20, 2025, at 15:30, Amit Langote <amitlangote09@gmail.com> =
wrote:
>=20
> =
<v3-0004-Use-pruning-aware-locking-in-cached-plans.patch><v3-0005-Add-test=
-exercising-prep-cleanup-on-cached-plan-i.patch><v3-0002-Introduce-Executo=
rPrep-and-refactor-executor-star.patch><v3-0006-Make-SQL-function-executor=
-track-ExecutorPrep-sta.patch><v3-0003-Reuse-partition-pruning-results-in-=
parallel-worke.patch><v3-0001-Refactor-partition-pruning-initialization-fo=
r-cla.patch>


0001 splits creations of es_part_prune_states into a new function =
ExecCreatePartitionPruneStates(). With that, you are trying to make the =
code clearer as you stated in the commit comment. However, the new =
function is not called, meaning 0001 is not self-contained, feels =
unusual to me according to the patches I have reviewed so far. I would =
suggest have ExecDoInitialPruning() call =
ExecCreatePartitionPruneStates() when es_part_prune_states is still =
NIL., so that current logic is unchanged, and 0001 can be pushed =
independently.

0002 moves check permission etc logic from InitPlan() to the new =
function ExecutorPrep(). The commit message says =E2=80=9Cexecutor setup =
logic unchanged=E2=80=9D. Because in old code, before permission check, =
there was no PushActiveSnapshot(), but in the patch, before check =
permission, PushActiveSnapshot() is done, which may introduce different =
behavior, I just wonder why PushActiveSnapshot() is added?

Actually, I am still trying to understand 0002-0004, it would take me =
some time to fully understand the patch. I=E2=80=99d raise the above =
comments first. I will continue reviewing this patch tomorrow.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/