Jacob Champion <
jchampion@timescale.com> writes:
> Hi Joe, here's a partial review:
Thanks so much for the review!
> I'm new to this code, but I agree that the use of `type` and the
> lookahead are not particularly obvious/intuitive. At the very least,
> they'd need some more explanation in the code. Your boolean flag idea
> sounds reasonable, though.
I've updated the patch with the boolean flag idea. I think it's a
bit cleaner and more readable.
>> There is one more problem I noticed, but didn't fix. We allow multiple
>> "@" to be sprinkled anywhere in the input, even though the docs [0]
>> only allow it to appear at the beginning of the input.
>
> (No particular opinion on this.)
I looked into this a bit. The reason this works is because the date
time lexer filters out all punctuation. That's what allows us to parse
things like `SELECT date 'January 8, 1999';`. It's probably not worth
trying to be smarter about what punctuation we allow where, at least
for now. Maybe in the future we can exclude "@" from the punctuation
that get's filtered out.
> It looks like this patch needs a rebase for the CI, too, but there are
> no conflicts.
The attached patch is rebased against master.
Thanks,
Joe Koshakow