public inbox for [email protected]help / color / mirror / Atom feed
Re: Skip adding row-marks for non target tables when result relation is foreign table. 4+ messages / 3 participants [nested] [flat]
* Re: Skip adding row-marks for non target tables when result relation is foreign table. @ 2024-08-09 08:35 Etsuro Fujita <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Etsuro Fujita @ 2024-08-09 08:35 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: SAIKIRAN AVULA <[email protected]>; pgsql-hackers On Wed, May 22, 2024 at 10:13 AM Jeff Davis <[email protected]> wrote: > On Mon, 2024-05-06 at 23:10 +0100, SAIKIRAN AVULA wrote: > > Additionally, the commit afb9249d06f47d7a6d4a89fea0c3625fe43c5a5d, > > which introduced late locking for foreign tables, mentions that the > > benefits of late locking against a remote server are unclear, as the > > extra round trips required are likely to outweigh any potential > > concurrency improvements. > > The extra round trip only happens when EPQ finds a newer version of the > tuple, which should be the exceptional case. I'm not sure how this > balances out, but to me late locking still seems preferable. Early > locking is a huge performance hit in some cases (locking many more rows > than necessary). I might be missing something, but I think the extra round trip happens for each foreign row locked using the RefetchForeignRow() API in ExecLockRows(), so I think the overhead is caused in the normal case. > Early locking is also a violation of the documentation here: > > "When a locking clause appears at the top level of a SELECT query, the > rows that are locked are exactly those that are returned by the query; > in the case of a join query, the rows locked are those that contribute > to returned join rows." Yeah, but I think this holds true for SELECT queries postgres_fdw sends to the remote side. :) Best regards, Etsuro Fujita ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Skip adding row-marks for non target tables when result relation is foreign table. @ 2024-08-15 00:56 Jeff Davis <[email protected]> parent: Etsuro Fujita <[email protected]> 0 siblings, 1 reply; 4+ messages in thread From: Jeff Davis @ 2024-08-15 00:56 UTC (permalink / raw) To: Etsuro Fujita <[email protected]>; +Cc: SAIKIRAN AVULA <[email protected]>; pgsql-hackers On Fri, 2024-08-09 at 17:35 +0900, Etsuro Fujita wrote: > I might be missing something, but I think the extra round trip > happens > for each foreign row locked using the RefetchForeignRow() API in > ExecLockRows(), so I think the overhead is caused in the normal case. Is there any sample code that implements late locking for a FDW? I'm not quite clear on how it's supposed to work. Regards, Jeff Davis ^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: Skip adding row-marks for non target tables when result relation is foreign table. @ 2024-08-15 11:45 Etsuro Fujita <[email protected]> parent: Jeff Davis <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Etsuro Fujita @ 2024-08-15 11:45 UTC (permalink / raw) To: Jeff Davis <[email protected]>; +Cc: SAIKIRAN AVULA <[email protected]>; pgsql-hackers On Thu, Aug 15, 2024 at 9:56 AM Jeff Davis <[email protected]> wrote: > Is there any sample code that implements late locking for a FDW? I'm > not quite clear on how it's supposed to work. See the patch in [1]. It would not apply to HEAD anymore, though. Best regards, Etsuro Fujita [1] https://www.postgresql.org/message-id/[email protected] ^ permalink raw reply [nested|flat] 4+ messages in thread
* [PATCH] Add hook for plugins to acquire sample rows during ANALYZE @ 2026-05-29 12:35 Samba Siva <[email protected]> 0 siblings, 0 replies; 4+ messages in thread From: Samba Siva @ 2026-05-29 12:35 UTC (permalink / raw) - Introduced AcquireSampleRowsFunc_hook for extensions to override row sampling. - Updated analyze.c to utilize the hook if registered. - Added tests to ensure ANALYZE completes without errors with the new hook. --- doc/src/sgml/xfunc.sgml | 10 ++++++++++ src/backend/commands/analyze.c | 24 ++++++++++++++++++++---- src/include/commands/vacuum.h | 11 +++++++++++ src/test/regress/expected/analyze.out | 20 ++++++++++++++++++++ src/test/regress/parallel_schedule | 1 + src/test/regress/sql/analyze.sql | 24 ++++++++++++++++++++++++ 6 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 src/test/regress/expected/analyze.out create mode 100644 src/test/regress/sql/analyze.sql diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 1eb5abffd8..f1ef025ae2 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -4203,4 +4203,14 @@ supportfn(internal) returns internal To create such conditions, the support function must implement the <literal>SupportRequestIndexCondition</literal> request type. </para> + + <para> + The hook variable <varname>AcquireSampleRowsFunc_hook</varname> allows extensions + to override the row sampling function during <command>ANALYZE</command> for regular + heap relations. This is useful for extensions/systems which implement distributed + databases and want to sample rows from remote nodes instead of the local heap. + The hook function fills the provided <literal>rows[]</literal> buffer with at most + <literal>targrows</literal> heap tuples and sets <literal>*totalrows</literal> to the + estimated total live row count of the relation. + </para> </sect1> diff --git a/src/backend/commands/analyze.c b/src/backend/commands/analyze.c index 4fffb76e55..3560acdff1 100644 --- a/src/backend/commands/analyze.c +++ b/src/backend/commands/analyze.c @@ -74,6 +74,8 @@ int default_statistics_target = 100; static MemoryContext anl_context = NULL; static BufferAccessStrategy vac_strategy; +/* Hook for plugins to acquire sample rows for ANALYZE */ +AcquireSampleRowsFunc_hook_type AcquireSampleRowsFunc_hook = NULL; static void do_analyze_rel(Relation onerel, VacuumParams *params, List *va_cols, @@ -188,8 +190,15 @@ analyze_rel(Oid relid, RangeVar *relation, if (onerel->rd_rel->relkind == RELKIND_RELATION || onerel->rd_rel->relkind == RELKIND_MATVIEW) { - /* Regular table, so we'll use the regular row acquisition function */ - acquirefunc = acquire_sample_rows; + /* + * Regular table, so we'll use the regular row acquisition function. + * If a plugin has registered a hook to acquire sample rows, use it; + * otherwise use the default function. + */ + if (AcquireSampleRowsFunc_hook) + acquirefunc = AcquireSampleRowsFunc_hook; + else + acquirefunc = acquire_sample_rows; /* Also get regular table's size */ relpages = RelationGetNumberOfBlocks(onerel); } @@ -1467,8 +1476,15 @@ acquire_inherited_sample_rows(Relation onerel, int elevel, if (childrel->rd_rel->relkind == RELKIND_RELATION || childrel->rd_rel->relkind == RELKIND_MATVIEW) { - /* Regular table, so use the regular row acquisition function */ - acquirefunc = acquire_sample_rows; + /* + * Regular table, so use the regular row acquisition function. + * If a plugin has registered a hook to acquire sample rows, use it; + * otherwise use the default function. + */ + if (AcquireSampleRowsFunc_hook) + acquirefunc = AcquireSampleRowsFunc_hook; + else + acquirefunc = acquire_sample_rows; relpages = RelationGetNumberOfBlocks(childrel); } else if (childrel->rd_rel->relkind == RELKIND_FOREIGN_TABLE) diff --git a/src/include/commands/vacuum.h b/src/include/commands/vacuum.h index bc37a80dc7..146f936861 100644 --- a/src/include/commands/vacuum.h +++ b/src/include/commands/vacuum.h @@ -21,6 +21,7 @@ #include "catalog/pg_class.h" #include "catalog/pg_statistic.h" #include "catalog/pg_type.h" +#include "foreign/fdwapi.h" #include "parser/parse_node.h" #include "storage/buf.h" #include "storage/lock.h" @@ -113,6 +114,9 @@ typedef void (*AnalyzeAttrComputeStatsFunc) (VacAttrStatsP stats, int samplerows, double totalrows); +/* Hook type for plugins to acquire sample rows for ANALYZE */ +typedef AcquireSampleRowsFunc AcquireSampleRowsFunc_hook_type; + typedef struct VacAttrStats { /* @@ -334,6 +338,13 @@ extern PGDLLIMPORT int vacuum_cost_limit; extern PGDLLIMPORT int64 parallel_vacuum_worker_delay_ns; +/* + * Hook for plugins to override row sampling during ANALYZE. + * Also applies to child relations of partitioned/inherited tables. + * See acquire_sample_rows() in src/backend/commands/analyze.c. + */ +extern PGDLLIMPORT AcquireSampleRowsFunc_hook_type AcquireSampleRowsFunc_hook; + /* in commands/vacuum.c */ extern void ExecVacuum(ParseState *pstate, VacuumStmt *vacstmt, bool isTopLevel); extern void vacuum(List *relations, VacuumParams *params, diff --git a/src/test/regress/expected/analyze.out b/src/test/regress/expected/analyze.out new file mode 100644 index 0000000000..5bb6bbc45b --- /dev/null +++ b/src/test/regress/expected/analyze.out @@ -0,0 +1,20 @@ +-- Test AcquireSampleRowsFunc_hook +-- Usually this would be tested via a C extension. +-- Here we just confirm this does not break the existing ANALYZE code +-- by verifying that ANALYZE completes without error. +CREATE TABLE employees ( + id SERIAL PRIMARY KEY, + name TEXT, + department TEXT, + salary NUMERIC +); +INSERT INTO employees + SELECT + i, + 'Employee ' || i, + 'Department ' || (i % 5), + (i % 100) * 1000 + 50000 + FROM generate_series(1, 1000) i; +-- Should complete without error +ANALYZE employees; +DROP TABLE employees; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index e1e0c54019..71978ef858 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -94,6 +94,7 @@ test: vacuum_parallel # Run this alone, because concurrent DROP TABLE would make non-superuser # "ANALYZE;" fail with "relation with OID $n does not exist". test: maintain_every +test: analyze # no relation related tests can be put in this group test: publication subscription diff --git a/src/test/regress/sql/analyze.sql b/src/test/regress/sql/analyze.sql new file mode 100644 index 0000000000..e0bc84e9f9 --- /dev/null +++ b/src/test/regress/sql/analyze.sql @@ -0,0 +1,24 @@ +-- Test AcquireSampleRowsFunc_hook +-- Usually this would be tested via a C extension. +-- Here we just confirm this does not break the existing ANALYZE code +-- by verifying that ANALYZE completes without error. + +CREATE TABLE employees ( + id SERIAL PRIMARY KEY, + name TEXT, + department TEXT, + salary NUMERIC +); + +INSERT INTO employees + SELECT + i, + 'Employee ' || i, + 'Department ' || (i % 5), + (i % 100) * 1000 + 50000 + FROM generate_series(1, 1000) i; + +-- Should complete without error +ANALYZE employees; + +DROP TABLE employees; -- 2.50.1 (Apple Git-155) ------=_Part_741589_445982825.1782438414800-- ^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-05-29 12:35 UTC | newest] Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2024-08-09 08:35 Re: Skip adding row-marks for non target tables when result relation is foreign table. Etsuro Fujita <[email protected]> 2024-08-15 00:56 ` Jeff Davis <[email protected]> 2024-08-15 11:45 ` Etsuro Fujita <[email protected]> 2026-05-29 12:35 [PATCH] Add hook for plugins to acquire sample rows during ANALYZE Samba Siva <[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