public inbox for [email protected]
help / color / mirror / Atom feedFrom: Tomas Vondra <[email protected]>
To: Rahila Syed <[email protected]>
To: Alvaro Herrera <[email protected]>
Cc: PostgreSQL-development <[email protected]>
Subject: Re: Column Filtering in Logical Replication
Date: Mon, 12 Jul 2021 14:54:26 +0200
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH2L28sZj2Bj=73qZrXy69y6w5BRKKpunV3P7tnXYQKPbA11gw@mail.gmail.com>
References: <CAH2L28vddB_NFdRVpuyRBJEBWjz4BSyTB=_ektNRH8NJ1jf95g@mail.gmail.com>
<[email protected]>
<CAH2L28sZj2Bj=73qZrXy69y6w5BRKKpunV3P7tnXYQKPbA11gw@mail.gmail.com>
On 7/12/21 11:38 AM, Rahila Syed wrote:
> Hi Alvaro,
>
> Thank you for comments.
>
> The patch adds a function get_att_num_by_name; but we have a lsyscache.c
> function for that purpose, get_attnum. Maybe that one should be used
> instead?
>
> Thank you for pointing that out, I agree it makes sense to reuse the
> existing function.
> Changed it accordingly in the attached patch.
>
>
> get_tuple_columns_map() returns a bitmapset of the attnos of the columns
> in the given list, so its name feels wrong. I propose
> get_table_columnset(). However, this function is invoked for every
> insert/update change, so it's going to be far too slow to be usable. I
> think you need to cache the bitmapset somewhere, so that the function is
> only called on first use. I didn't look very closely, but it seems that
> struct RelationSyncEntry may be a good place to cache it.
>
> Makes sense, changed accordingly.
>
To nitpick, I find "Bitmapset *att_list" a bit annoying, because it's
not really a list ;-)
FWIW "make check" fails for me with this version, due to segfault in
OpenTableLists. Apparenly there's some confusion - the code expects the
list to contain PublicationTable nodes, and tries to extract the
RangeVar from the elements. But the list actually contains RangeVar, so
this crashes and burns. See the attached backtrace.
I'd bet this is because the patch uses list of RangeVar in some cases
and list of PublicationTable in some cases, similarly to the "row
filtering" patch nearby. IMHO this is just confusing and we should
always pass list of PublicationTable nodes.
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Core was generated by `postgres: user regression [local] ALTER PUBLICATION '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00000000006be5b5 in OpenTableList (tables=0x27b35a0) at publicationcmds.c:520
520 bool recurse = rv->inh;
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.32-6.fc33.x86_64
(gdb) bt
#0 0x00000000006be5b5 in OpenTableList (tables=0x27b35a0) at publicationcmds.c:520
#1 0x00000000006be137 in AlterPublicationTables (stmt=0x27b35f8, rel=0x73e26485ec80, tup=0x2877c98) at publicationcmds.c:375
#2 0x00000000006be458 in AlterPublication (stmt=0x27b35f8) at publicationcmds.c:464
#3 0x00000000009762b4 in ProcessUtilitySlow (pstate=0x2877b80, pstmt=0x27b3968, queryString=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x27b3a58, qc=0x7ffec211be70) at utility.c:1809
#4 0x0000000000974968 in standard_ProcessUtility (pstmt=0x27b3968, queryString=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x27b3a58, qc=0x7ffec211be70) at utility.c:1049
#5 0x0000000000973bb8 in ProcessUtility (pstmt=0x27b3968, queryString=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x27b3a58, qc=0x7ffec211be70) at utility.c:527
#6 0x0000000000972890 in PortalRunUtility (portal=0x2816ff0, pstmt=0x27b3968, isTopLevel=true, setHoldSnapshot=false, dest=0x27b3a58, qc=0x7ffec211be70) at pquery.c:1147
#7 0x0000000000972af4 in PortalRunMulti (portal=0x2816ff0, isTopLevel=true, setHoldSnapshot=false, dest=0x27b3a58, altdest=0x27b3a58, qc=0x7ffec211be70) at pquery.c:1304
#8 0x000000000097202d in PortalRun (portal=0x2816ff0, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x27b3a58, altdest=0x27b3a58, qc=0x7ffec211be70) at pquery.c:786
#9 0x000000000096bc4f in exec_simple_query (query_string=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;") at postgres.c:1214
#10 0x000000000097015a in PostgresMain (argc=1, argv=0x7ffec211c100, dbname=0x27deed8 "regression", username=0x27deeb8 "user") at postgres.c:4486
#11 0x00000000008ad08d in BackendRun (port=0x27d7fc0) at postmaster.c:4507
#12 0x00000000008aca0c in BackendStartup (port=0x27d7fc0) at postmaster.c:4229
#13 0x00000000008a8fc0 in ServerLoop () at postmaster.c:1745
#14 0x00000000008a888b in PostmasterMain (argc=8, argv=0x27ac590) at postmaster.c:1417
#15 0x00000000007ab2b0 in main (argc=8, argv=0x27ac590) at main.c:209
Attachments:
[text/plain] crash.txt (2.6K, ../[email protected]/2-crash.txt)
download | inline:
Core was generated by `postgres: user regression [local] ALTER PUBLICATION '.
Program terminated with signal SIGSEGV, Segmentation fault.
#0 0x00000000006be5b5 in OpenTableList (tables=0x27b35a0) at publicationcmds.c:520
520 bool recurse = rv->inh;
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.32-6.fc33.x86_64
(gdb) bt
#0 0x00000000006be5b5 in OpenTableList (tables=0x27b35a0) at publicationcmds.c:520
#1 0x00000000006be137 in AlterPublicationTables (stmt=0x27b35f8, rel=0x73e26485ec80, tup=0x2877c98) at publicationcmds.c:375
#2 0x00000000006be458 in AlterPublication (stmt=0x27b35f8) at publicationcmds.c:464
#3 0x00000000009762b4 in ProcessUtilitySlow (pstate=0x2877b80, pstmt=0x27b3968, queryString=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;", context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x27b3a58, qc=0x7ffec211be70) at utility.c:1809
#4 0x0000000000974968 in standard_ProcessUtility (pstmt=0x27b3968, queryString=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x27b3a58, qc=0x7ffec211be70) at utility.c:1049
#5 0x0000000000973bb8 in ProcessUtility (pstmt=0x27b3968, queryString=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;", readOnlyTree=false, context=PROCESS_UTILITY_TOPLEVEL, params=0x0, queryEnv=0x0, dest=0x27b3a58, qc=0x7ffec211be70) at utility.c:527
#6 0x0000000000972890 in PortalRunUtility (portal=0x2816ff0, pstmt=0x27b3968, isTopLevel=true, setHoldSnapshot=false, dest=0x27b3a58, qc=0x7ffec211be70) at pquery.c:1147
#7 0x0000000000972af4 in PortalRunMulti (portal=0x2816ff0, isTopLevel=true, setHoldSnapshot=false, dest=0x27b3a58, altdest=0x27b3a58, qc=0x7ffec211be70) at pquery.c:1304
#8 0x000000000097202d in PortalRun (portal=0x2816ff0, count=9223372036854775807, isTopLevel=true, run_once=true, dest=0x27b3a58, altdest=0x27b3a58, qc=0x7ffec211be70) at pquery.c:786
#9 0x000000000096bc4f in exec_simple_query (query_string=0x27b2ae0 "ALTER PUBLICATION testpub_default SET TABLE testpub_tbl1;") at postgres.c:1214
#10 0x000000000097015a in PostgresMain (argc=1, argv=0x7ffec211c100, dbname=0x27deed8 "regression", username=0x27deeb8 "user") at postgres.c:4486
#11 0x00000000008ad08d in BackendRun (port=0x27d7fc0) at postmaster.c:4507
#12 0x00000000008aca0c in BackendStartup (port=0x27d7fc0) at postmaster.c:4229
#13 0x00000000008a8fc0 in ServerLoop () at postmaster.c:1745
#14 0x00000000008a888b in PostmasterMain (argc=8, argv=0x27ac590) at postmaster.c:1417
#15 0x00000000007ab2b0 in main (argc=8, argv=0x27ac590) at main.c:209
view thread (167+ 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]
Subject: Re: Column Filtering in Logical Replication
In-Reply-To: <[email protected]>
* 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