[ 
https://issues.apache.org/jira/browse/CALCITE-6071?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17783433#comment-17783433
 ] 

Mihai Budiu edited comment on CALCITE-6071 at 11/7/23 12:54 AM:
----------------------------------------------------------------

There is a storm of changes, and it took me a long time to make them all. But 
intermediate versions of this PR had many more. I worked hard to reduce the 
changes too.
This change is in some sense best-effort: we propagate the information when we 
have it, but when not we have to use a ZERO.
Whether the information propagated is good enough can be told only by users of 
the system who may complain that there is no sufficient information about 
runtime errors.
The only way to make sure it always works is to:
- create a backend which signals such errors with position information
- write many negative tests (which cause runtime errors) for this backend

I have thought about having a list of "safe" functions which never require 
position information and checking that the position is ZERO only for these 
cases in makeCall.
I will work next on adding a test case. I can do it in the same PR. But making 
sure that all code generated in Calcite uses this information will probably be 
a series of separate issues and PRs.


was (Author: JIRAUSER295926):
There is a storm of changes, and it took me a long time to make them all. But 
intermediate versions of this PR had many more. I worked hard to reduce the 
changes too.
This change is in some sense best-effort: we propagate the information when we 
have it, but when not we have to use a ZERO.
Whether the information propagated is good enough can be told only by users of 
the system who may complain that there is no sufficient information about 
runtime errors.
The only way to make sure it always works is to:
- create a backend which signals such errors with position information
- write many negative tests (which cause runtime errors) for this backend
I have thought about having a list of "safe" functions which never require 
position information and checking that the position is ZERO only for these 
cases in makeCall.
I will work next on adding a test case. I can do it in the same PR. But making 
sure that all code generated in Calcite uses this information will probably be 
a series of separate issues and PRs.

> RexCall should carry source position information for runtime error reporting
> ----------------------------------------------------------------------------
>
>                 Key: CALCITE-6071
>                 URL: https://issues.apache.org/jira/browse/CALCITE-6071
>             Project: Calcite
>          Issue Type: Improvement
>          Components: core
>    Affects Versions: 1.35.0
>            Reporter: Mihai Budiu
>            Priority: Minor
>              Labels: pull-request-available
>
> Currently runtime errors, such as division by zero, cannot report the 
> position in the original source code where the original operator was. RexNode 
> does not carry such information. By adding the required information in 
> RexNode we can report much better runtime errors to users.
> Here is an additional comment from [~jhyde] on the dev mailing list:
> The RexCall to such throwable functions would have a hidden ‘pos’ parameter. 
> The parameter should be populated at Sql-to-rel time, and if it is not 
> present, SqlOperator.validareRexOperands should throw. Associating the 
> position with the particular function call seems to me much more robust than 
> associating it with the RelNode that contains the call.
>  



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to