Skip to content

Conversation

@slinkydeveloper
Copy link
Contributor

No description provided.

Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
@slinkydeveloper
Copy link
Contributor Author

We should backport this to 1.15 branch as well

@flinkbot
Copy link
Collaborator

flinkbot commented Mar 21, 2022

CI report:

Bot commands The @flinkbot bot supports the following commands:
  • @flinkbot run azure re-run the last Azure build

return CodeGenUtils.boxedTypeTermForType(t);
}
switch (t.getTypeRoot()) {
case BOOLEAN:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi, I think it would be better to use CodeGenUtils.primitiveTypeTermForType instead, what do you think?
Maybe we need to check isPrimitiveNullable first when t is nullable, if it is true, we should use primitiveType instead of boxedType.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed a change, also adding another test to make sure this works. Does it looks good now?

Signed-off-by: slinkydeveloper <francescoguard@gmail.com>
Copy link
Contributor

@wenlong88 wenlong88 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for the update, I left a few more comment.

@twalthr twalthr closed this in 7f3fa54 Mar 23, 2022
twalthr pushed a commit that referenced this pull request Mar 23, 2022
cun8cun8 pushed a commit to cun8cun8/flink that referenced this pull request Mar 28, 2022
cun8cun8 pushed a commit to cun8cun8/flink that referenced this pull request Mar 28, 2022
cun8cun8 pushed a commit to cun8cun8/flink that referenced this pull request Mar 28, 2022
JasonLeeCoding pushed a commit to JasonLeeCoding/flink that referenced this pull request May 27, 2022
zstraw pushed a commit to zstraw/flink that referenced this pull request Jul 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants