Hi Stefan, Thank you for your comment. I updated webrev:
webrev: http://cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.01/ submit-hs: mach5-one-ysuenaga-JDK-8199519-20180327-0652-16322 Thanks, Yasumasa 2018-03-27 0:03 GMT+09:00 Stefan Johansson <stefan.johans...@oracle.com>: > Hi Yasumasa, > > On 2018-03-22 11:35, Yasumasa Suenaga wrote: >> >> Hi all, >> >> Please review this change: >> >> JBS: https://bugs.openjdk.java.net/browse/JDK-8199519 >> webrev: cr.openjdk.java.net/~ysuenaga/JDK-8199519/webrev.00/ > > The fix seems to make things to work as expected. Manually tested it and > Mach5 also looks good. > > I have some comments regarding the patch. I think 'forcibly' should be > rename to something more descriptive. Naming is never easy but I think > 'required' would be better, as in, this column is required and not allowed > to print '-'. That would also render the code in ExpressionResolver.java to > be: > return new Literal(isRequired ? 0.0d : Double.NaN); > I think that also better explains why we return 0 instead of NaN. > > I would also like to see the forcibly/required state moved into the > Expression it self, that way we don't have to pass it around but can instead > do: > return new Literal(e.isRequired() ? 0.0d : Double.NaN); > > Thanks, > Stefan > > >> >> After JDK-8153333, some jstat tests are failed because GCT in jstat output >> is dash (-) if garbage collector is not concurrent collector e.g. Serial GC. >> I fixed that GCT can be calculated correctly. >> >> This change has been tested on Mach5 by Stefan. >> >> >> Thanks, >> >> Yasumasa > >
