[bug #1389] Resolve processing Nested column type in combination with AggregateFunction

[mdonkers]

Summary

Resolves #1389

Skipping of whitespace is prevented when processing a section within brackets.
I had to change some other tests that were failing now, but don't think it would break anything else. Not sure why whitespace is removed in the first place?

Checklist

Delete items not relevant to your PR:

• Unit and integration tests covering the common scenarios were added
• A human-readable description of the changes was provided to include in CHANGELOG
• For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

[zhicwu]

Thank you @mdonkers for the fix, it looks good to me. The build failure is unrelated, but I'll need to fix that before merging this PR. I'll get back to this next week.

[mdonkers]

Thanks!

One question from my side, why are whitespaces removed in the first place? I've seen e.g. " a, b c" in one of the tests that gets translated to the params Arrays.asList("a", "bc")
Would there be situations where the whitespace not being removed could now lead to issues? I guess if there is something like ( a, b c) it would result in Arrays.asList("a", "b c"). I contemplated making the readParameters smarter to look back at Nested or some other keywords to detect name-type combinations but that would be a much bigger change.

Just trying to understand if my PR could have unexpected consequences somewhere else.

[mdonkers]

Hi @zhicwu, I expect this one slipped through the cracks.
Do you have an expectation on when this could be merged?

[zhicwu]

Hi @mdonkers, sorry for taking so long. I need to play with the change by a few more tests. Will try to get it merged tonight.

--
Apparently I also missed above questions :<

One question from my side, why are whitespaces removed in the first place? I've seen e.g. " a, b c" in one of the tests that gets translated to the params Arrays.asList("a", "bc"). Would there be situations where the whitespace not being removed could now lead to issues?

I was hoping to generate consistent parsing result, regardless the type info was returned from server(strictly formatted), or hand-crafted. This way, we can build a reliable cache layer later if needed.

I contemplated making the readParameters smarter to look back at Nested or some other keywords to detect name-type combinations but that would be a much bigger change.

Yes, that will be too much :)