Re: [PATCH] Do not use StdRdOptions in Access Methods - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] Do not use StdRdOptions in Access Methods
Date
Msg-id 20191031074939.GF2530@paquier.xyz
Whole thread Raw
In response to Re: [PATCH] Do not use StdRdOptions in Access Methods  (Amit Langote <amitlangote09@gmail.com>)
Responses Re: [PATCH] Do not use StdRdOptions in Access Methods  (Amit Langote <amitlangote09@gmail.com>)
List pgsql-hackers
On Thu, Oct 31, 2019 at 04:38:55PM +0900, Amit Langote wrote:
> On Wed, Oct 30, 2019 at 12:11 PM Michael Paquier <michael@paquier.xyz> wrote:
> This sentence sounds wrong, because the result structure doesn't
> contain values in text-array format.  Individual values in the struct
> would be in their native format (C bool for RELOPT_TYPE_BOOL, options,
> etc.).
>
> Maybe we don't need this sentence, because the first line already says
> we return a struct.

Let's remove it then.

> This breakage seems to have to do with the fact that the definition of
> DummyIndexOptions struct and the entries of relopt_parse_elt table
> don't agree?
>
> These are the last two members of DummyIndexOptions struct:
>
> @@ -126,7 +126,7 @@ create_reloptions_table(void)
>   NULL, &validate_string_option,
>   AccessExclusiveLock);
>   di_relopt_tab[5].optname = "option_string_null";
> - di_relopt_tab[5].opttype = RELOPT_TYPE_STRING;
> + di_relopt_tab[5].opttype = RELOPT_TYPE_INT;
>   di_relopt_tab[5].offset = offsetof(DummyIndexOptions,
>      option_string_null_offset);
>  }
>
> test passes.
>
> But maybe this Assert isn't all that robust, so I'm happy to take it out.

This one should remain a string reloption, and what's stored in the
structure is an offset to get the string.  See for example around
RelationHasCascadedCheckOption before it got switched to an enum in
773df88 regarding the way to get the value.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Re: Allow cluster_name in log_line_prefix
Next
From: Amit Langote
Date:
Subject: Re: update ALTER TABLE with ATTACH PARTITION lock mode