Re: Allow to_date() and to_timestamp() to accept localized names

On Wed, Jan 15, 2020 at 11:20 AM Arthur Zakirov <zaartur(at)gmail(dot)com> wrote:

>
> I have some couple picky notes.
>
>
Thanks for the review.

> > + if (name_len != norm_len)
> > + pfree(norm_name);
>
> I'm not sure here. Is it save to assume that if it was allocated a new
> norm_name name_len and norm_len always will differ?
>

Good call, that check can be more robust.

>
> > static const char *const months_full[] = {
> > "January", "February", "March", "April", "May", "June", "July",
> > - "August", "September", "October", "November", "December", NULL
> > + "August", "September", "October", "November", "December"
> > };
>
> Unfortunately I didn't see from the patch why it was necessary to remove
> last null element from months_full, days_short, adbc_strings,
> adbc_strings_long and other arrays. I think it is not good because
> unnecessarily increases the patch and adds code like the following:
>
> > + from_char_seq_search(&value, &s, months,
> localized_names,
> > +
> ONE_UPPER, MAX_MON_LEN, n, have_error,
> > +
> lengthof(months_full));
>
> Here it passes "months" from datetime.c to from_char_seq_search() and
> calculates its length using "months_full" array from formatting.c.
>

With the introduction of seq_search_localized() that can be avoided,
minimizing code churn.

Please, find attached a version addressing the above mentioned.

Regards,

Juan José Santamaría Flecha

>
>