Re: currawong is not a happy animal - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: currawong is not a happy animal
Date
Msg-id CAA4eK1L5GSnyQ+-+qvisp-oFJYKQ+NJA=AqWhCtMyG9Zh_oL2A@mail.gmail.com
Whole thread Raw
In response to Re: currawong is not a happy animal  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: currawong is not a happy animal  (Andrew Dunstan <andrew@dunslane.net>)
Re: currawong is not a happy animal  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
On Sat, Jan 18, 2014 at 2:48 AM, Andrew Dunstan <andrew@dunslane.net> wrote:
>
> On 01/17/2014 03:15 PM, Tom Lane wrote:
>
>> The other possibility I was contemplating is that "export a const
>> variable" doesn't actually work for some reason.  We're not in the habit
>> of doing that elsewhere, so I don't find that theory outlandish.  Perhaps
>> it could be fixed by adding PGDLLIMPORT to the extern, but on the whole
>> I'd rather avoid the technique altogether.

Is there any specific reason why adding PGDLLIMPORT for exporting
const variables is not good, when we are already doing for other variables
like InterruptHoldoffCount, CritSectionCount?

On searching in code, I found that for few const variables we do
extern PGDLLIMPORT. For example:
extern PGDLLIMPORT const int NumScanKeywords;
extern PGDLLIMPORT const char *debug_query_string;

>> The least-unlike-other-Postgres-code approach would be to go ahead and
>> export the struct so that the size computation could be provided as a
>> #define in the same header.  Robert stated a couple days ago that he
>> didn't foresee much churn in this struct, so that doesn't seem
>> unacceptable.
>>
>> Another possibility is to refactor so that testing an allocation request
>> against shm_mq_minimum_size is the responsibility of storage/ipc/shm_mq.c,
>> not some random code in a contrib module.  It's not immediately apparent
>> to me why it's good code modularization to have a contrib module
>> responsible for checking sizes based on the sizeof a struct it's not
>> supposed to have any access to.
>
> Or maybe we could expose the value via a function rather than a const
> variable.

All of above suggested alternatives will fix this problem. However after
fixing this problem, when I ran the test further it crashed the bgworker
and I found that reason was there are some other variables
(ImmediateInterruptOK, MyBgworkerEntry) used in test module without
PGDLLIMPORT.

After adding PGDLLIMPORT to variables (ImmediateInterruptOK,
MyBgworkerEntry, shm_mq_minimum_size) both the tests defined in
contrib module passed.


Attached patch fixes the problems related to test_shm_mq for me.


With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment

pgsql-hackers by date:

Previous
From: David Rowley
Date:
Subject: Re: [PATCH] Negative Transition Aggregate Functions (WIP)
Next
From: Robert Haas
Date:
Subject: Re: ALTER TABLESPACE ... MOVE ALL TO ...