Skip to content

CALC-1803 Related, Cleaned up the flow control logic#499

Closed
axeisghost wants to merge 2 commits intoapache:masterfrom
axeisghost:flow-control-fix
Closed

CALC-1803 Related, Cleaned up the flow control logic#499
axeisghost wants to merge 2 commits intoapache:masterfrom
axeisghost:flow-control-fix

Conversation

@axeisghost
Copy link
Copy Markdown

Code coverity found out problem about flow control, mention in comments at CALC-1803

//fallthrough
break;
default:
rowBuilder.set(i, s);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why did you remove this line "rowBuilder.set(i, s);"? We still want to set the value if the type is not numeric.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Oh, It is totally my bad. I haven't touched this part for a while, so I kinda forget the original purpose of default branch.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I modified your code so that exceptional conditions throw or return, and non-exceptional conditions all go through the same 'rowBuilder.set(i, s)'. See julianhyde@cbbe627, and let me know if there's a problem. Otherwise I'll commit shortly.

@axeisghost
Copy link
Copy Markdown
Author

added the line back

@axeisghost
Copy link
Copy Markdown
Author

Thank you. It is good. Thanks for helping me fix the logic.

@asfgit asfgit closed this in cbbe627 Jul 18, 2017
7mming7 pushed a commit to Kyligence/calcite that referenced this pull request Jan 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants