Skip to content

DYN-2273 - Fix crash by compiling null input to null AST node#10127

Merged
aparajit-pratap merged 4 commits intoDynamoDS:masterfrom
aparajit-pratap:dyn-2273
Nov 13, 2019
Merged

DYN-2273 - Fix crash by compiling null input to null AST node#10127
aparajit-pratap merged 4 commits intoDynamoDS:masterfrom
aparajit-pratap:dyn-2273

Conversation

@aparajit-pratap
Copy link
Contributor

@aparajit-pratap aparajit-pratap commented Nov 11, 2019 •
edited
Loading

Purpose

JIRA: https://jira.autodesk.com/browse/DYN-2273

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning, and are documented in the API Changes document.

Reviewers

@DynamoDS/dynamo

@aparajit-pratap aparajit-pratap mentioned this pull request Nov 11, 2019
Closed
@QilongTang
Copy link
Contributor

QilongTang commented Nov 12, 2019

@aparajit-pratap , my memory fading. Can you describe null node a bit?

@aparajit-pratap aparajit-pratap changed the title DYN-2273 - Fix crash by compiling null input to null node DYN-2273 - Fix crash by compiling null input to null AST node Nov 12, 2019
@aparajit-pratap
Copy link
Contributor Author

aparajit-pratap commented Nov 12, 2019

@QilongTang by nullnode, I meant Null AST node, sorry for not making that clear before. We have a special AST node to represent null. In the case where an input AST is null, we would rather propagate the null than throw an exception either at the time of AST compilation or at runtime, which was what was happening before.

@QilongTang
Copy link
Contributor

QilongTang commented Nov 12, 2019

@aparajit-pratap Thank you, makes sense. LGTM

@QilongTang QilongTang added the LGTM Looks good to me label Nov 12, 2019
@aparajit-pratap aparajit-pratap merged commit 89465ff into DynamoDS:master Nov 13, 2019
@aparajit-pratap aparajit-pratap deleted the dyn-2273 branch November 13, 2019 14:41
@aparajit-pratap aparajit-pratap mentioned this pull request Nov 14, 2019
Merged
7 tasks
QilongTang pushed a commit that referenced this pull request Feb 13, 2020
@aparajit-pratap @QilongTang
DYN-2273 - Fix crash by compiling null input to null AST node (#10127)
2551cd2 
* fix crash by compiling null input to null node

* add test

* update recorded test to reflect null value of downstream node

* remove unused usings
@QilongTang QilongTang mentioned this pull request Feb 13, 2020
Merged
7 tasks
QilongTang added a commit that referenced this pull request Feb 13, 2020
@QilongTang @aparajit-pratap
DYN-2273 - Fix crash by compiling null input to null AST node (#10127) (
e1c204d 
#10391)

* fix crash by compiling null input to null node

* add test

* update recorded test to reflect null value of downstream node

* remove unused usings

Co-authored-by: aparajit-pratap <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

LGTM Looks good to me

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@aparajit-pratap @QilongTang