В письме от 9 февраля 2018 18:45:29 пользователь Alvaro Herrera написал:
> If this patch gets in, I wonder if there are any external modules that > use actual strings. An hypothetical example would be something like a > SSL cipher list; it needs to be somewhat free-form that an enum would > not cut it. If there are such modules, then even if we remove all > existing in-core use cases we should keep the support code for strings. I did not remove string option support from the code. It might be needed later. > Maybe we need some in-core user to verify the string case still works. > A new module in src/test/modules perhaps? This sound as a good idea. I am too do not feel really comfortable with that this string options possibility exists, but is not tested. I'll have a look there, it it will not require a great job, I will add tests for string options there. > On the other hand, if we can > find no use for these string reloptions, maybe we should just remove the > support, since as I recall it's messy enough. No, the implementation of string options is quite good. And may be needed later. > > Possible flaws: > > > > 1. I've changed error message from 'Valid values are "XXX", "YYY" and > > "ZZZ".' to 'Valid values are "XXX", "YYY", "ZZZ".' to make a code a bit > > simpler. If it is not acceptable, please let me know, I will add "and" to > > the string. > I don't think we care about this, but is this still the case if you use > a stringinfo? May be not. I'll try to do it better. > > 2. Also about the string with the list of acceptable values: the code that > > creates this string is inside parse_one_reloption function now. > > I think you could save most of that mess by using appendStringInfo and > friends. Thanks. I will rewrite this part using these functions. That was really helpful. > I don't much like the way you've represented the list of possible values > for each enum. I think it'd be better to have a struct that represents > everything about each value (string name and C symbol. I actually do not like it this way too. I would prefer one structure, not two lists. But I did not find way how to do it in one struct. How to gave have string value and C symbol in one structure, without defining C symbols elsewhere. Otherwise it will be two lists again. I do not have a lot of hardcore C development experience, so I can miss something. Can you provide an example of the structure you are talking about? > Maybe the > numerical value too if that's needed, but is it? I suppose all code > should use the C symbol only, so why do we care about the numerical > value?). It is comfortable to have numerical values when debugging. I like to write something like elog(WARNING,"buffering_mode=%i",opt.buffering_mode); to check that everything works as expected. Such cases is the only reason to keep numerical value. -- Do code for fun.
signature.asc
Description: This is a digitally signed message part.
