public inbox for [email protected]help / color / mirror / Atom feed
Re: [PATCH] analyze: move elevel calculation into do_analyze_rel() 3+ messages / 3 participants [nested] [flat]
* Re: [PATCH] analyze: move elevel calculation into do_analyze_rel() @ 2026-03-31 17:56 Lev Nikolaev <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Lev Nikolaev @ 2026-03-31 17:56 UTC (permalink / raw) To: pgsql-hackers Hi hackers, I reworked the patch slightly. Since do_analyze_rel() is static, removing the extra elevel argument is local to analyze.c only. No behavioral change is intended. -- Best regards, Lev Nikolaev, Tantor Labs LLC, https://tantorlabs.com/ 30.03.2026 16:03, Лев Николаев пишет: > > Hi hackers, > > While reading analyze.c in current master, I noticed a small cleanup > opportunity in the ANALYZE code path. > > Currently, analyze_rel() computes elevel from params.options, but does > not use it directly. It only passes the value down to > do_analyze_rel(). At the same time, do_analyze_rel() already receives > params and derives verbose from params.options for its own purposes. > > The attached patch moves the elevel calculation into do_analyze_rel() > and removes the extra elevel argument from its declaration, > definition, and call sites. > > No behavioral change is intended here. The logging level selection > remains the same; the calculation is just moved closer to the actual > use sites. > > Comments and feedback would be appreciated. > > -- > > Best regards, > > Lev Nikolaev, > > Tantor Labs LLC, > > https://tantorlabs.com/ > Attachments: [text/x-patch] 0001-analyze-move-elevel-calculation-into-do_analyze_rel.patch (3.1K, 3-0001-analyze-move-elevel-calculation-into-do_analyze_rel.patch) download | inline diff: From 5c1f08a6f7c24aa26b96fa6f96bc53149621c386 Mon Sep 17 00:00:00 2001 From: Lev Nikolaev <[email protected]> Date: Mon, 30 Mar 2026 11:15:47 +0000 Subject: [PATCH] analyze: move elevel calculation into do_analyze_rel() analyze_rel() computes elevel from params.options, but does not use it directly and only passes it to do_analyze_rel(). Since do_analyze_rel() already receives params and derives verbose from params.options, compute elevel there instead and remove the extra function argument. No behavioral change intended. --- src/backend/commands/analyze.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index eeed91be266..3c565b83475 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -78,7 +78,7 @@ static BufferAccessStrategy vac_strategy; static void do_analyze_rel(Relation onerel, const VacuumParams params, List *va_cols, AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, - bool inh, bool in_outer_xact, int elevel); + bool inh, bool in_outer_xact); static void compute_index_stats(Relation onerel, double totalrows, AnlIndexData *indexdata, int nindexes, HeapTuple *rows, int numrows, @@ -111,16 +111,9 @@ analyze_rel(Oid relid, RangeVar *relation, BufferAccessStrategy bstrategy) { Relation onerel; - int elevel; AcquireSampleRowsFunc acquirefunc = NULL; BlockNumber relpages = 0; - /* Select logging level */ - if (params.options & VACOPT_VERBOSE) - elevel = INFO; - else - elevel = DEBUG2; - /* Set up static variables */ vac_strategy = bstrategy; @@ -253,14 +246,14 @@ analyze_rel(Oid relid, RangeVar *relation, */ if (onerel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) do_analyze_rel(onerel, params, va_cols, acquirefunc, - relpages, false, in_outer_xact, elevel); + relpages, false, in_outer_xact); /* * If there are child tables, do recursive ANALYZE. */ if (onerel->rd_rel->relhassubclass) do_analyze_rel(onerel, params, va_cols, acquirefunc, relpages, - true, in_outer_xact, elevel); + true, in_outer_xact); /* * Close source relation now, but keep lock so that no one deletes it @@ -283,15 +276,15 @@ analyze_rel(Oid relid, RangeVar *relation, static void do_analyze_rel(Relation onerel, const VacuumParams params, List *va_cols, AcquireSampleRowsFunc acquirefunc, - BlockNumber relpages, bool inh, bool in_outer_xact, - int elevel) + BlockNumber relpages, bool inh, bool in_outer_xact) { int attr_cnt, tcnt, i, ind; Relation *Irel; - int nindexes; + int nindexes, + elevel; bool verbose, instrument, hasindex; @@ -316,6 +309,13 @@ do_analyze_rel(Relation onerel, const VacuumParams params, PgStat_Counter startwritetime = 0; verbose = (params.options & VACOPT_VERBOSE) != 0; + + /* Select logging level */ + if (verbose) + elevel = INFO; + else + elevel = DEBUG2; + instrument = (verbose || (AmAutoVacuumWorkerProcess() && params.log_analyze_min_duration >= 0)); if (inh) -- 2.34.1 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] analyze: move elevel calculation into do_analyze_rel() @ 2026-04-01 23:29 Andreas Karlsson <[email protected]> parent: Lev Nikolaev <[email protected]> 0 siblings, 1 reply; 3+ messages in thread From: Andreas Karlsson @ 2026-04-01 23:29 UTC (permalink / raw) To: Lev Nikolaev <[email protected]>; pgsql-hackers On 3/31/26 7:56 PM, Lev Nikolaev wrote: > I reworked the patch slightly. This small cleanup makes sense to me since do_analyze_rel() already looks at if VACOPT_VERBOSE is set meaning related code is grouped closer after this small refactoring. The patch no longer applied so I rebased it. Andreas Attachments: [text/x-patch] v3-0001-analyze-move-elevel-calculation-into-do_analyze_r.patch (3.1K, 2-v3-0001-analyze-move-elevel-calculation-into-do_analyze_r.patch) download | inline diff: From 916f7ec1cd4194abc8c3ac1e330415867f0c5374 Mon Sep 17 00:00:00 2001 From: Lev Nikolaev <[email protected]> Date: Mon, 30 Mar 2026 11:15:47 +0000 Subject: [PATCH v3] analyze: move elevel calculation into do_analyze_rel() analyze_rel() computes elevel from params.options, but does not use it directly and only passes it to do_analyze_rel(). Since do_analyze_rel() already receives params and derives verbose from params.options, compute elevel there instead and remove the extra function argument. No behavioral change intended. --- src/backend/commands/analyze.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 49a5cdf579c..07de28c3cec 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -78,7 +78,7 @@ static BufferAccessStrategy vac_strategy; static void do_analyze_rel(Relation onerel, const VacuumParams *params, List *va_cols, AcquireSampleRowsFunc acquirefunc, BlockNumber relpages, - bool inh, bool in_outer_xact, int elevel); + bool inh, bool in_outer_xact); static void compute_index_stats(Relation onerel, double totalrows, AnlIndexData *indexdata, int nindexes, HeapTuple *rows, int numrows, @@ -111,16 +111,9 @@ analyze_rel(Oid relid, RangeVar *relation, BufferAccessStrategy bstrategy) { Relation onerel; - int elevel; AcquireSampleRowsFunc acquirefunc = NULL; BlockNumber relpages = 0; - /* Select logging level */ - if (params->options & VACOPT_VERBOSE) - elevel = INFO; - else - elevel = DEBUG2; - /* Set up static variables */ vac_strategy = bstrategy; @@ -253,14 +246,14 @@ analyze_rel(Oid relid, RangeVar *relation, */ if (onerel->rd_rel->relkind != RELKIND_PARTITIONED_TABLE) do_analyze_rel(onerel, params, va_cols, acquirefunc, - relpages, false, in_outer_xact, elevel); + relpages, false, in_outer_xact); /* * If there are child tables, do recursive ANALYZE. */ if (onerel->rd_rel->relhassubclass) do_analyze_rel(onerel, params, va_cols, acquirefunc, relpages, - true, in_outer_xact, elevel); + true, in_outer_xact); /* * Close source relation now, but keep lock so that no one deletes it @@ -283,15 +276,15 @@ analyze_rel(Oid relid, RangeVar *relation, static void do_analyze_rel(Relation onerel, const VacuumParams *params, List *va_cols, AcquireSampleRowsFunc acquirefunc, - BlockNumber relpages, bool inh, bool in_outer_xact, - int elevel) + BlockNumber relpages, bool inh, bool in_outer_xact) { int attr_cnt, tcnt, i, ind; Relation *Irel; - int nindexes; + int nindexes, + elevel; bool verbose, instrument, hasindex; @@ -316,6 +309,13 @@ do_analyze_rel(Relation onerel, const VacuumParams *params, PgStat_Counter startwritetime = 0; verbose = (params->options & VACOPT_VERBOSE) != 0; + + /* Select logging level */ + if (verbose) + elevel = INFO; + else + elevel = DEBUG2; + instrument = (verbose || (AmAutoVacuumWorkerProcess() && params->log_analyze_min_duration >= 0)); if (inh) -- 2.47.3 ^ permalink raw reply [nested|flat] 3+ messages in thread
* Re: [PATCH] analyze: move elevel calculation into do_analyze_rel() @ 2026-04-02 11:14 Etsuro Fujita <[email protected]> parent: Andreas Karlsson <[email protected]> 0 siblings, 0 replies; 3+ messages in thread From: Etsuro Fujita @ 2026-04-02 11:14 UTC (permalink / raw) To: Andreas Karlsson <[email protected]>; +Cc: Lev Nikolaev <[email protected]>; pgsql-hackers On Thu, Apr 2, 2026 at 8:29 AM Andreas Karlsson <[email protected]> wrote: > On 3/31/26 7:56 PM, Lev Nikolaev wrote: > > I reworked the patch slightly. > > This small cleanup makes sense to me since do_analyze_rel() already > looks at if VACOPT_VERBOSE is set meaning related code is grouped closer > after this small refactoring. > > The patch no longer applied so I rebased it. Sorry, but -1 from me because this change would lead to doing the same setup repeatedly when analyzing inheritance trees, which is not great. Best regards, Etsuro Fujita ^ permalink raw reply [nested|flat] 3+ messages in thread
end of thread, other threads:[~2026-04-02 11:14 UTC | newest] Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2026-03-31 17:56 Re: [PATCH] analyze: move elevel calculation into do_analyze_rel() Lev Nikolaev <[email protected]> 2026-04-01 23:29 ` Andreas Karlsson <[email protected]> 2026-04-02 11:14 ` Etsuro Fujita <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox