On 10/03/16 02:18, Kouhei Kaigai wrote: > >> I am not sure I like the fact that we have this EXTNODENAME_MAX_LEN and >> now the CUSTOM_NAME_MAX_LEN with the same length and also they are both >> same lenght as NAMEDATALEN I wonder if this shouldn't be somehow >> squished to less defines. >> > Hmm. I just followed the manner in extensible.c, because this label was > initially NAMEDATALEN, then Robert changed it with EXTNODENAME_MAX_LEN. > I guess he avoid to apply same label on different entities - NAMEDATALEN > is a limitation for NameData type, but identifier of extensible-node and > custom-scan node are not restricted by. >
Makes sense. >> Also in RegisterCustomScanMethods >> + Assert(strlen(methods->CustomName) <= CUSTOM_NAME_MAX_LEN); >> >> Shouldn't this be actually "if" with ereport() considering this is >> public API and extensions can pass anything there? (for that matter same >> is true for RegisterExtensibleNodeMethods but that's already committed). >> > Hmm. I don't have clear answer which is better. The reason why I put > Assert() here is that only c-binary extension uses this interface, thus, > author will fix up the problem of too long name prior to its release. > Of course, if-with-ereport() also informs extension author the name is > too long. > One downside of Assert() may be, it makes oversight if --enable-cassert > was not specified. > Well that's exactly my problem, this should IMHO throw error even without --enable-cassert. It's not like it's some performance sensitive API where if would be big problem, ensuring correctness of the input is more imporant here IMHO. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
