NPE in blink planner code gen

[Benchao Li]

hi all,

We are using 1.9.0 blink planner, and find flink will throw NPE when we use
the following SQL:

```
create table source {
  age int,
  id varchar
};
select *case when age < 20 then cast(id as bigint) else 0 end* from source;
```

After debugging the Janino generated code, I find that NPE's reason is that
`BinaryStringUtil.toLong` returns `null`, and we assign this result to a
`long` field.

Then a tried old planner, it throw a `java.lang.NumberFormatException` when
casting a blank string to int.
And also tried other illegal casting in blink, which come out to be `null`.

So, here is my question:
Obviously, this is a bug in blink planner, and we should fix that. But we
have two ways to fix this:
1, make behavior of cast behave like before, which produces `null`,
2, change the behavior of blink planner to align with old planner, which
produces `NumberFormatException`.



Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:[phone]
Email: [hidden email]; [hidden email]

[Leonard Xu]

Hi, Benchao, thank you for your report.
It looks egacy planner and blink planner have different behavior.
Could you create an issue in https://issues.apache.org/jira/browse/FLINK <https://issues.apache.org/jira/browse/FLINK> ?

> On Dec 15, 2019, at 16:17, Benchao Li <[hidden email]> wrote:
>
> hi all,
>
> We are using 1.9.0 blink planner, and find flink will throw NPE when we use
> the following SQL:
>
> ```
> create table source {
>  age int,
>  id varchar
> };
> select *case when age < 20 then cast(id as bigint) else 0 end* from source;
> ```
>
> After debugging the Janino generated code, I find that NPE's reason is that
> `BinaryStringUtil.toLong` returns `null`, and we assign this result to a
> `long` field.
>
> Then a tried old planner, it throw a `java.lang.NumberFormatException` when
> casting a blank string to int.
> And also tried other illegal casting in blink, which come out to be `null`.
>
> So, here is my question:
> Obviously, this is a bug in blink planner, and we should fix that. But we
> have two ways to fix this:
> 1, make behavior of cast behave like before, which produces `null`,
> 2, change the behavior of blink planner to align with old planner, which
> produces `NumberFormatException`.
>
>
>
> Benchao Li
> School of Electronics Engineering and Computer Science, Peking University
> Tel:[phone]
> Email: [hidden email]; [hidden email]

[Jingsong Li]

Hi Benchao,

Thanks for your reporting.
As Leonard said, you can create an issue in JIRA. We can go on discussing
in JIRA.
The answer is #1, Blink's behavior ensures that the job runs as much as
possible without interrupting it, so null is returned here to make it
possible to continue running.

In JIRA, you can describe more detailed, like input data, program detail.
And you can verify master or 1.10 code too.

Best,
Jingsong Lee

On Mon, Dec 16, 2019 at 9:23 AM Leonard Xu <[hidden email]> wrote:

> Hi, Benchao, thank you for your report.
> It looks egacy planner and blink planner have different behavior.
> Could you create an issue in https://issues.apache.org/jira/browse/FLINK ?
>
>
>
> On Dec 15, 2019, at 16:17, Benchao Li <[hidden email]> wrote:
>
> hi all,
>
> We are using 1.9.0 blink planner, and find flink will throw NPE when we use
> the following SQL:
>
> ```
> create table source {
>  age int,
>  id varchar
> };
> select *case when age < 20 then cast(id as bigint) else 0 end* from source;
> ```
>
> After debugging the Janino generated code, I find that NPE's reason is that
> `BinaryStringUtil.toLong` returns `null`, and we assign this result to a
> `long` field.
>
> Then a tried old planner, it throw a `java.lang.NumberFormatException` when
> casting a blank string to int.
> And also tried other illegal casting in blink, which come out to be `null`.
>
> So, here is my question:
> Obviously, this is a bug in blink planner, and we should fix that. But we
> have two ways to fix this:
> 1, make behavior of cast behave like before, which produces `null`,
> 2, change the behavior of blink planner to align with old planner, which
> produces `NumberFormatException`.
>
>
>
> Benchao Li
> School of Electronics Engineering and Computer Science, Peking University
> Tel:[phone]
> Email: [hidden email] <[hidden email]>; [hidden email]
>
>
>

--
Best, Jingsong Lee

[Benchao Li]

Hi Jingsong, Leonard,

Thanks for your response, I'll created an issue (
https://issues.apache.org/jira/browse/FLINK-15266) to track this.
And further discussions can be moved to JIRA.

Jingsong Li <[hidden email]> 于2019年12月16日周一 上午10:17写道：

> Hi Benchao,
>
> Thanks for your reporting.
> As Leonard said, you can create an issue in JIRA. We can go on discussing
> in JIRA.
> The answer is #1, Blink's behavior ensures that the job runs as much as
> possible without interrupting it, so null is returned here to make it
> possible to continue running.
>
> In JIRA, you can describe more detailed, like input data, program detail.
> And you can verify master or 1.10 code too.
>
> Best,
> Jingsong Lee
>
> On Mon, Dec 16, 2019 at 9:23 AM Leonard Xu <[hidden email]> wrote:
>
>> Hi, Benchao, thank you for your report.
>> It looks egacy planner and blink planner have different behavior.
>> Could you create an issue in https://issues.apache.org/jira/browse/FLINK
>>  ?
>>
>>
>>
>> On Dec 15, 2019, at 16:17, Benchao Li <[hidden email]> wrote:
>>
>> hi all,
>>
>> We are using 1.9.0 blink planner, and find flink will throw NPE when we
>> use
>> the following SQL:
>>
>> ```
>> create table source {
>>  age int,
>>  id varchar
>> };
>> select *case when age < 20 then cast(id as bigint) else 0 end* from
>> source;
>> ```
>>
>> After debugging the Janino generated code, I find that NPE's reason is
>> that
>> `BinaryStringUtil.toLong` returns `null`, and we assign this result to a
>> `long` field.
>>
>> Then a tried old planner, it throw a `java.lang.NumberFormatException`
>> when
>> casting a blank string to int.
>> And also tried other illegal casting in blink, which come out to be
>> `null`.
>>
>> So, here is my question:
>> Obviously, this is a bug in blink planner, and we should fix that. But we
>> have two ways to fix this:
>> 1, make behavior of cast behave like before, which produces `null`,
>> 2, change the behavior of blink planner to align with old planner, which
>> produces `NumberFormatException`.
>>
>>
>>
>> Benchao Li
>> School of Electronics Engineering and Computer Science, Peking University
>> Tel:[phone]
>> Email: [hidden email] <[hidden email]>; [hidden email]
>>
>>
>>
>
> --
> Best, Jingsong Lee
>

--

Benchao Li
School of Electronics Engineering and Computer Science, Peking University
Tel:[phone]
Email: [hidden email]; [hidden email]