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
