cgivre edited a comment on pull request #2118:
URL: https://github.com/apache/drill/pull/2118#issuecomment-736608577


   > My notes
   > 
   > * The new test code covers both the text-valued formula and number-valued 
formula cases.
   > * I looked for some simple, fast alternatives to the exception-driven 
testing of whether a cell's content can be parsed as a double but I found 
nothing compelling.
   
   I didn't either.  Unfortunately, the streaming reader does place some limits 
on what is possible. 
   
   > * On line 467 of ExcelBatchReader.java `numericValue` is a throw-away.  
Possibly a style improvement is not to assign the value of 
`cell.getNumericCellValue()` to anything, or to assign it to something explicit 
like `double throwAway = ...`
   > 
   > In ExcelBatchReader.java
   
   I'll make that change.  Really all we want to do is see whether it CAN be 
read.  We don't actually use the value at that point. 
   
   


----------------------------------------------------------------
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.

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to