> On 12 February 2018 at 12:49, Arthur Zakirov <a.zaki...@postgrespro.ru> wrote: > > Yes, I somehow missed it. I changed the behaviour of separator > characters in format string. They are greedy now and eat whitespaces > before a separator character in an input string. I suppose that there > should be no such problems as in [1].
Thanks. I have a few minor complains about some noise: * On line 52 there is a removed empty line, without any other changes around * On line 177 there is a new commented out line of code. I assume it's not an explanation or something and we don't need it, am I right? Also, I spotted one more difference between this patch and Oracle. In the situation, when a format string doesn't have anything meaningful, with the patch we've got: SELECT to_timestamp('2000 + JUN', '/'); to_timestamp --------------------------------- 0001-01-01 00:00:00+00:53:28 BC (1 row) SELECT to_timestamp('2000 + JUN', ' '); to_timestamp --------------------------------- 0001-01-01 00:00:00+00:53:28 BC (1 row) And Oracle complains about this: SELECT to_timestamp('2000 + JUN', ' /') FROM dual ORA-01830: date format picture ends before converting entire input string SELECT to_timestamp('2000 + JUN', ' ') FROM dual ORA-01830: date format picture ends before converting entire input string It's sort of corner case, but anyway maybe you would be interested to handle it. >> About usage of locale dependent functions e.g. `isalpha`. Yes, looks like >> it's better to have locale-agnostic implementation, but then I'm confused - all >> functions except `isdigit`/`isxdigit` are locale-dependent, including >> `isspace`, which is also in use. > > I returned is_separator_char() function for now. Thanks. Answering my own question about `isspace`, I finally noticed, that this function was used before the patch, and there were no complains - so probably it's fine.