On Thu, Oct 31, 2019 at 4:49 PM Michael Paquier <mich...@paquier.xyz> wrote: > On Thu, Oct 31, 2019 at 04:38:55PM +0900, Amit Langote wrote: > > On Wed, Oct 30, 2019 at 12:11 PM Michael Paquier <mich...@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.
Removed in the attached. > > 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. I see. I didn't know that about STRING options when writing that Assert, so it was indeed broken to begin with. v5 attached. Thanks, Amit
build_reloptions-v5.patch
Description: Binary data
