Re: proposal: function parse_ident

Hi

2015-11-17 1:49 GMT+01:00 Marko Tiikkaja <marko(at)joh(dot)to>:

> On 9/11/15 12:25 PM, Pavel Stehule wrote:
>
>> new update of parse_ident function patch
>>
>
> Nice! I've certainly wanted something like this a number of times.
>
> Some comments about the v2 of the patch:
>
> - The patch doesn't apply anymore, so it should be rebased.
>

done

> - The docs don't even try and explain what the "strictmode" parameter
> does.

fixed

- The comment before the parse_ident function is not up to date anymore,
> since "the rest" was removed from the interface.

fixed

- I can't immediately grep for any uses of do { .. } while (true) from
> our code base. AFAICT the majority look like for (;;); I see no reason
> not to be consistent here.
>

fixed

- What should happen if the input is a string like
> 'one.two.three.four.five.six'? Do we want to accept input like that?
>

I don't see the reason why not. It is pretty simple to count fields in
result array and raise error later. The application has better information
about expected and valid numbers. But any opinion in this question should
be valid. I have not strong position here.

> - I haven't reviewed the actual parsing code carefully, but didn't we
> already have a function which splits identifiers up? I of course can't
> find one with my grepping right now, so I might be wrong.
>

There is: SplitIdentifierString or textToQualifiedNameList in varlena.c. My
first patch was based on these functions. But I cannot to use it.

1. major reason: The buildin parser is based on searching the dot "." and
doesn't search any disallowed identifiers chars. So there is not possible
to implement non strict mode - find last char of last identifier and ignore
other.
2. minor reason: little bit more precious diagnostics - buildin routines
returns only true (valid) and false (invalid).

Regards

Pavel

>
>
> .m
>