2015-11-17 1:49 GMT+01:00 Marko Tiikkaja <marko@joh.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).
