Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed two bugs in FastDeserializerGenerator #46

Merged

Conversation

volauvent
Copy link
Collaborator

Fixed two bugs which caused FastDeserializer generation failure -

  1. Fixed null exception when updating actual exceptions for already generated record method
  2. Corrected iteration order of value action symbols for nested Map field

@gaojieliu @FelixGV @radai-rosenblatt @pravk03

@volauvent volauvent force-pushed the fix-fast-deserializer-generator branch from 7dbd483 to 7ba8013 Compare May 4, 2020 04:08
@@ -698,7 +700,8 @@ private void processMap(JVar mapSchemaVar, final String name, final Schema mapSc

if (action.getShouldRead()) {
Symbol valuesActionSymbol = null;
for (Symbol symbol : action.getSymbol().production) {
for (int i = action.getSymbol().production.length - 1; i >= 0; --i) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Could you explain more about this change?
Why sequence matters in this situation?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It's for nested Map deserializer generation. The "symbol.production" generated by ResolvingGrammarGenerator is in reversed order, and the REPEATER symbol holds the sequence of the symbols that forms the production for it.

Resolving grammar of nested Map is like:

- symbol/"map-end"/"REPEATER"                ----    1st layer Map
  - production
    - symbol/"map-end"/"REPEATER"            ----    1st layer Map
    - symbol/"map-end"/"REPEATER"            ----    2nd layer Map
    - symbol/"map-start"/"TERMINAL"          ----    2nd layer Map
- symbol/"map-start"/"TERMINAL"              ----    1st layer Map

The fastDeserializer generator tries to proceed as a depth-first, left-to-right traversal of the schema. So when it is processing the 2nd layer of Map (production of 1st layer Map Symbol), we need to iterate production list in reverse order to get the correct 2nd layer "map-end" symbol.

Copy link
Collaborator

Choose a reason for hiding this comment

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

needs at least some comment in the code :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Agreed and good catch!
This logic seems tricky, and You can put your explanation as the javadoc for this section.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done. Added comments for the change.

1. fixed null exception when updating actual exceptions for already generated record method;
2. corrected value action symbols iteration order for nested Map;
@gaojieliu gaojieliu merged commit 84432ee into linkedin:master May 6, 2020
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.

None yet

3 participants