gianm commented on PR #20756: URL: https://github.com/apache/druid/pull/20756#issuecomment-6471668294
> ## 🟡 Changes recommended > > The static review found one functional gap: the shorthand parser rejects negative array indices even though equivalent JSON_VALUE paths are supported and the documentation promises the same JSONPath elements. Add signed-index support before merging. I'm inclined to leave this out, because the grammar for `:` field access comes from upstream Calcite and this would be a divergence that could contribute to future merge conflicts. Generally we want to keep those to a minimum. The negative index functionality doesn't seem worth it. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
