Skip to content

Fix index validation for nested $and - #1014

Merged
willholley merged 1 commit into
apache:masterfrom
willholley:mango_field_order
Nov 23, 2017
Merged

Fix index validation for nested $and#1014
willholley merged 1 commit into
apache:masterfrom
willholley:mango_field_order

Conversation

@willholley

@willholley willholley commented Nov 22, 2017

Copy link
Copy Markdown
Member

Overview

mango_selector:has_required_fields checks that a list of
indexed fields is covered by a given selector. The implementation
recurses through the selector, tracking fields that encounters.

Unfortunately, this skipped peers of combination operators. For
example,

"selector": {
	"$and":[
		"$and":[
			"A": "foo"
		],
		"$and":[
			"B": "bar"
		]
	]
}

would skip the first nested "$and" operator and only return "B"
as a covered field.

This commit explicitly handles this situation (the only combination
operator we care about is $and), so for the above selector we
would correctly identify "A" and "B" as covered fields.

Testing recommendations

Run the test suite. Test that Query selectors with nested $and operators correctly use json indexes that should cover them.

Related Issues or Pull Requests

Checklist

  • Code is written and works correctly;
  • Changes are covered by tests;
  • Documentation reflects the changes;

mango_selector:has_required_fields checks that a list of
indexed fields is covered by a given selector. The implementation
recurses through the selector, tracking fields that encounters.

Unfortunately, this skipped peers of combination operators. For
example,

"selector": {
	"$and":[
		"$and":[
			"A": "foo"
		],
		"$and":[
			"B": "bar"
		]
	]
}

would skip the first nested "$and" operator and only return "B"
as a covered field.

This commit explicitly handles this situation (the only combination
operator we care about is $and), so for the above selector we
would correctly indentify "A" and "B" as covered fields.
@willholley

Copy link
Copy Markdown
Member Author

Thanks to @afinne for identifying the issue. The original bug report showed that the index / query combination at https://pastebin.com/WiQ5Edgu failed.

@tonysun83

Copy link
Copy Markdown
Contributor

+1

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