mihaibudiu opened a new pull request, #3506:
URL: https://github.com/apache/calcite/pull/3506

   This PR adds source position (`SqlParserPos`) information to two kinds of 
nodes:
   - RexCall
   - AggregateCall
   
   Not all RexCall nodes and AggregateCall operations need to have meaningful 
source position information. The most useful nodes are the ones that could lead 
to runtime errors. So operations like addition (which can overflow) or casts 
(which can fail when the target type is too narrow) need to carry such 
information. Nodes where the operation cannot fail dynamically (e.g., MAX, 
comparisons, or IS_NULL) may have the source position set to ZERO.
   
   In this PR I have carefully audited the entire code base and tried to thread 
the flow of the position information everywhere, but there are a few places 
where I couldn't figure out where it should come from. There are, for example, 
a few casts inserted by the rewrite rules which have no direct correspondence 
to source constructs. Such casts have now a TODO comment wondering about their 
runtime safety.
   
   In the process I have also removed a few `@Deprecated` functions which would 
have required a signature change. I figured that since they are deprecated 
there is no point to update them, since no one can call the variant with the 
new signature.
   
   For methods where there are many calls where the position information may 
legitimately be ZERO I have left variants of the original methods without 
source position information (e.g., see RexBuilder.makeCall)
   
   Currently there are no uses of this information, but I have inspected the IR 
with a debugger after optimizations to check that the information is present. A 
future commit (part of this PR or another one) should add an example runtime 
error that displays this information. This is not trivial to do for the Calcite 
Linq4j-based executor, so I haven't done it yet.
   
   None of this should affect the behavior of any of the compiled programs, but 
I noticed that it's easy to mess up something in practice a few times. 
Currently no tests need to change.


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

Reply via email to