wojustme commented on pull request #2529: URL: https://github.com/apache/calcite/pull/2529#issuecomment-923585216
> > > Can you use a varchar constant, not integer constant in the group by list. > > > Hi @yanlin-Lynn > > > Should we need case of `group by varchar literal`? > > > In query's relnode, `group by varchar literal` is the same to `group by integer literal`. > > > They are all belong to the pattern of AggregateOnCalc. > > As a general comment, test cases should be crafted with the expected behaviour in mind, not the concrete implementation. > > It might seem trivial in the current state of the implementation, but tomorrow a change could break this assumption (maybe simply by mistake), and the test case would capture it. > > Of course there must be a reasonable chance for this to happen, I am not saying that this is necessarily the case here. > > > Can you use a varchar constant, not integer constant in the group by list. > > > Hi @yanlin-Lynn > > > Should we need case of `group by varchar literal`? > > > In query's relnode, `group by varchar literal` is the same to `group by integer literal`. > > > They are all belong to the pattern of AggregateOnCalc. > > As a general comment, test cases should be crafted with the expected behaviour in mind, not the concrete implementation. > > It might seem trivial in the current state of the implementation, but tomorrow a change could break this assumption (maybe simply by mistake), and the test case would capture it. > > Of course there must be a reasonable chance for this to happen, I am not saying that this is necessarily the case here. Ok, got it. Thanks for for your advice. @asolimando -- 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]
