From: Michael Paquier <michael@paquier.xyz>
To: Thomas Munro <thomas.munro@gmail.com>
Cc: Nikolay Shaplov <dhyan@nataraj.su>
Cc: Dent John <denty@qqdd.eu>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Robert Haas <robertmhaas@gmail.com>
Subject: Re: [PATCH] src/test/modules/dummy_index -- way to test reloptions from inside of access method
Date: Thu, 19 Sep 2019 17:32:03 +0900
Message-ID: <20190919083203.GC21144@paquier.xyz> (raw)
In-Reply-To: <CA+hUKG+8294zbN_k=QDgcW2TFia5TqsTR8nJLyYU+_C8vUTmrg@mail.gmail.com>
References: <17071942.m9zZutALE6@x200m>
<20190319070913.GE2899@paquier.xyz>
<1562184.o3gKksHyEI@x200m>
<6EED2D0C-6FC9-4958-BA6C-632E19210531@QQdd.eu>
<CA+hUKG+7RWOvnvZ1H+e2iUfkXNA17RYO1upoGZm8Tsaet7HbDQ@mail.gmail.com>
<1862640.9qQ6WrGjfI@x200m>
<CA+hUKG+8294zbN_k=QDgcW2TFia5TqsTR8nJLyYU+_C8vUTmrg@mail.gmail.com>
On Thu, Sep 19, 2019 at 10:51:09AM +1200, Thomas Munro wrote:
> Let's see if I can see this on my Mac... yep, with "make -C
> src/test/modules/dummy_index_am directory check" I see a similar
> failure with "ERROR: unrecognized lock mode: 2139062143". I changed
> that to a PANIC and got a core file containing this stack:
A simple make check on the module reproduces the failure, so that's
hard to miss.
From what I can see it is not a problem caused directly by this
module, specifically knowing that this test module is actually copying
what bloom is doing when declaring its reloptions. Take this example:
CREATE EXTENSION bloom;
CREATE TABLE tbloom AS
SELECT
(random() * 1000000)::int as i1,
(random() * 1000000)::int as i2,
(random() * 1000000)::int as i3,
(random() * 1000000)::int as i4,
(random() * 1000000)::int as i5,
(random() * 1000000)::int as i6
FROM
generate_series(1,100);
CREATE INDEX bloomidx ON tbloom USING bloom (i1,i2,i3)
WITH (length=80, col1=2, col2=2, col3=4);
ALTER INDEX bloomidx SET (length=100);
And then you get that:
ERROR: XX000: unrecognized lock mode: 2139062143
LOCATION: LockAcquireExtended, lock.c:756
So the options are registered in the relOpts array managed by
reloptions.c but the data is not properly initialized. Hence when
looking at the lock needed we have an option match, but the lock
number is incorrect, causing the failure. It looks like there is no
direct way to enforce the lockmode used for a reloption added via
add_int_reloption which does the allocation to add the option to
add_reloption(), but enforcing the value to be initialized fixes the
issue:
inline patch 1: src/backend/access/common/reloptions.c (+0/-0 lines) download inline-1.patch--- a/src/backend/access/common/reloptions.c+++ b/src/backend/access/common/reloptions.c@@ -658,6 +658,7 @@ allocate_reloption(bits32 kinds, int type, constchar *name, const char *desc)
newoption->kinds = kinds;
newoption->namelen = strlen(name);
newoption->type = type;
+ newoption->lockmode = AccessExclusiveLock;
MemoryContextSwitchTo(oldcxt);
I would think that initializing that to a sane default is something
that we should do anyway, or is there some trick allowing the
manipulation of relOpts I am missing? Changing the relopts APIs in
back-branches is a no-go of course, but we could improve that for
13~.
While reading through the code, I found some extra issues... Here are
some comments about them.
+++ b/src/test/modules/dummy_index_am/Makefile
@@ -0,0 +1,21 @@
+# contrib/bloom/Makefile
Incorrect copy-paste here.
+extern IndexBulkDeleteResult *dibulkdelete(IndexVacuumInfo *info,
+ IndexBulkDeleteResult *stats, IndexBulkDeleteCallback callback,
+ void *callback_state);
All the routines defining the index AM can just be static, so there is
no point to complicate dummy_index.h with most of its contents.
Some routines are missing a (void) in their declaration when the
routines have no arguments. This can cause warnings.
No sure I see the point of the various GUC with the use of WARNING
messages to check the sanity of the parameters. I find that awkward.
--
Michael
Attachments:
signature.asc (application/pgp-signature, 832B) download
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: pgsql-hackers@postgresql.org
Cc: michael@paquier.xyz, thomas.munro@gmail.com, dhyan@nataraj.su, denty@qqdd.eu, pgsql-hackers@lists.postgresql.org, alvherre@2ndquadrant.com, tgl@sss.pgh.pa.us, robertmhaas@gmail.com
Subject: Re: [PATCH] src/test/modules/dummy_index -- way to test reloptions from inside of access method
In-Reply-To: <20190919083203.GC21144@paquier.xyz>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox