Skip to content

[CALCITE-3316] Exception while deserializing LogicalCorrelate from json string#1433

Merged
hsyuan merged 1 commit intoapache:masterfrom
yanlin-Lynn:yanlin-correlate
Sep 12, 2019
Merged

[CALCITE-3316] Exception while deserializing LogicalCorrelate from json string#1433
hsyuan merged 1 commit intoapache:masterfrom
yanlin-Lynn:yanlin-correlate

Conversation

@yanlin-Lynn
Copy link
Contributor

NullPointerException and ClassCastException may happen while deserializing LogicalCorrelate from json string.
Please refer to CALCITE-3316 for the full stack trace of the exception.
This PR tries to fix it.

@hsyuan
Copy link
Member

hsyuan commented Sep 4, 2019

Sorry, my fault. I am wondering will changing correlation to correlationId cause compatibility issue? Your previous commit might make more sense in this regard. What do you think?

@yanlin-Lynn
Copy link
Contributor Author

Sorry, my fault. I am wondering will changing correlation to correlationId cause compatibility issue? Your previous commit might make more sense in this regard. What do you think?

Yes, compatibility issue might be a problem. I reset it to the previous commit.

@yanlin-Lynn
Copy link
Contributor Author

There are many duplicate code in RelWriterTest.
And I refine the code in this PR too.

Copy link
Member

@hsyuan hsyuan left a comment

Choose a reason for hiding this comment

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

+1

@hsyuan hsyuan added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Sep 7, 2019
@hsyuan hsyuan merged commit 507d383 into apache:master Sep 12, 2019
@yanlin-Lynn yanlin-Lynn deleted the yanlin-correlate branch September 13, 2019 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants