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

Fix panic when multiply assign #1257

Merged

Conversation

care0717
Copy link
Contributor

@care0717 care0717 commented Jun 24, 2022

I fix #1256

When multi documents exist, I've got panic because of expressionNode.Operation.Preferences = nil.
But I've got type assertion error instead of this issue when I remove it. So I make type assertion safe.

@care0717 care0717 changed the title hotfix: fix panic when multiply assign Fix panic when multiply assign Jun 24, 2022
@care0717
Copy link
Contributor Author

care0717 commented Jun 24, 2022

I'm wondering what kind of test cases to add or not.

Before I fix it, this test case is passed.

	{
		skipDoc:    true,
		document:   "a: 1\n---\nb: 2",
		expression: `. *= {"c": 3}`,
		expected: []string{
			"D0, P[], (!!map)::a: 1\nc: 3\n",
			"D1, P[], (!!map)::b: 2\nc: 3\n",
		},
	},

Unlike stream_evaluator, operator_test push all nodes to inputList before GetMatchingNodes. (maybe I think)

Let me know what you think.

@mikefarah
Copy link
Owner

Wow this is a really good find! You are right about stream_evaluator vs operator_test as well. I think the best way to test this case (for now) is to add an acceptance test. I'm happy to do that.

@mikefarah mikefarah merged commit 9b47a29 into mikefarah:master Jun 25, 2022
mikefarah added a commit that referenced this pull request Jun 25, 2022
@care0717 care0717 deleted the hotfix/fix-panic-multiply-assign branch June 25, 2022 02:34
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.

panic when mutiple assign to multi documents
2 participants