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

Compound filter condition that includes a relation panics #1808

Closed
islamaliev opened this issue Aug 23, 2023 · 1 comment · Fixed by #1855
Closed

Compound filter condition that includes a relation panics #1808

islamaliev opened this issue Aug 23, 2023 · 1 comment · Fixed by #1855
Assignees
Labels
area/query Related to the query component bug Something isn't working

Comments

@islamaliev
Copy link
Contributor

Describe the problem
When running a query with a compound filter like _and that includes a relation (i.e. filtering on a related type) it panics with this call stack:

--- FAIL: TestQueryFromSingleSideWithEqFilterOnRelatedType (0.01s)
panic: runtime error: index out of range [0] with length 0 [recovered]
	panic: runtime error: index out of range [0] with length 0

goroutine 9 [running]:
testing.tRunner.func1.2({0x105e033a0, 0x14000f422e8})
	/Users/islam/sdk/go1.20.7/src/testing/testing.go:1526 +0x1c8
testing.tRunner.func1()
	/Users/islam/sdk/go1.20.7/src/testing/testing.go:1529 +0x384
panic({0x105e033a0, 0x14000f422e8})
	/Users/islam/sdk/go1.20.7/src/runtime/panic.go:884 +0x204
github.com/sourcenetwork/defradb/core.(*DocumentMapping).FirstIndexOfName(...)
	/Users/islam/dev/sourcenetwork/defradb/core/doc.go:194
github.com/sourcenetwork/defradb/planner/mapper.toFilterMap({0x140001984d0?, 0x140001502a0?}, {0x105c8a660?, 0x140001500f0?}, 0x14000055200)
	/Users/islam/dev/sourcenetwork/defradb/planner/mapper/mapper.go:1156 +0x390
github.com/sourcenetwork/defradb/planner/mapper.toFilterMap({0x10565cf82, 0x3}, {0x105bdc920?, 0x1400014c1f8?}, 0x14000055200)
	/Users/islam/dev/sourcenetwork/defradb/planner/mapper/mapper.go:1131 +0x5ec
github.com/sourcenetwork/defradb/planner/mapper.ToFilter({0x140008af3d6?}, 0x6?)
	/Users/islam/dev/sourcenetwork/defradb/planner/mapper/mapper.go:1098 +0xcc
github.com/sourcenetwork/defradb/planner/mapper.toTargetable(0x0, 0x1400014b500, 0x14000055200)
	/Users/islam/dev/sourcenetwork/defradb/planner/mapper/mapper.go:1073 +0x64
github.com/sourcenetwork/defradb/planner/mapper.toSelect(0x3?, 0x3?, 0x1400014b500, {0x0?, 0x0?})
	/Users/islam/dev/sourcenetwork/defradb/planner/mapper/mapper.go:128 +0x310
github.com/sourcenetwork/defradb/planner/mapper.ToSelect({0x105efc6b0?, 0x14000198018?}, {0x105f0d5c0?, 0x14000b071f0?}, 0x14000150000?)
	/Users/islam/dev/sourcenetwork/defradb/planner/mapper/mapper.go:39 +0x68
github.com/sourcenetwork/defradb/planner.(*Planner).newPlan(0x140001501b0, {0x105cd48e0?, 0x1400014b500?})
	/Users/islam/dev/sourcenetwork/defradb/planner/planner.go:115 +0x264
github.com/sourcenetwork/defradb/planner.(*Planner).newPlan(0x140001501b0, {0x105bb2380?, 0x14000150000?})
	/Users/islam/dev/sourcenetwork/defradb/planner/planner.go:112 +0x19c
github.com/sourcenetwork/defradb/planner.(*Planner).newPlan(0x140001501b0, {0x105bb2480?, 0x1400012bcc0?})
	/Users/islam/dev/sourcenetwork/defradb/planner/planner.go:101 +0x134
github.com/sourcenetwork/defradb/planner.(*Planner).makePlan(0x18?, {0x105bb2480?, 0x1400012bcc0?})
	/Users/islam/dev/sourcenetwork/defradb/planner/planner.go:167 +0x28
github.com/sourcenetwork/defradb/planner.(*Planner).RunRequest(0x140005341f8?, {0x105efc6b0, 0x14000198018}, 0x1400012bcc0)
	/Users/islam/dev/sourcenetwork/defradb/planner/planner.go:473 +0x50
github.com/sourcenetwork/defradb/db.(*db).execRequest(0x14000b1d130, {0x105efc6b0?, 0x14000198018}, {0x1058373c9, 0xb7}, {0x105f0d5c0?, 0x14000b071f0})
	/Users/islam/dev/sourcenetwork/defradb/db/request.go:53 +0x3c8
github.com/sourcenetwork/defradb/db.(*implicitTxnDB).ExecRequest(0x14000538038, {0x105efc6b0, 0x14000198018}, {0x1058373c9, 0xb7})
	/Users/islam/dev/sourcenetwork/defradb/db/txn_db.go:45 +0x150
github.com/sourcenetwork/defradb/tests/integration.executeRequest(0x140004ae160, {{0x0, 0x0}, {0x0, 0x0}, {0x1058373c9, 0xb7}, {0x140001a1018, 0x1, 0x1}, ...})
	/Users/islam/dev/sourcenetwork/defradb/tests/integration/utils2.go:1293 +0xe8
github.com/sourcenetwork/defradb/tests/integration.executeTestCase({0x105efc6b0?, 0x14000198018}, 0x140007829c0, {0x140004b6440, 0x2, 0x2}, {{0x10582a409, 0x49}, {0x140000ae300, 0xb, ...}}, ...)
	/Users/islam/dev/sourcenetwork/defradb/tests/integration/utils2.go:383 +0xab8
github.com/sourcenetwork/defradb/tests/integration.ExecuteTestCase(0x105e14460?, {{0x10582a409, 0x49}, {0x140000ae300, 0xb, 0x10}})
	/Users/islam/dev/sourcenetwork/defradb/tests/integration/utils2.go:288 +0x340
github.com/sourcenetwork/defradb/tests/integration.ExecuteRequestTestCase(0x104869fdc?, {0x105835b65?, 0x104869eac?}, {0x14000266380?, 0x104749ae4?, 0x105c634e0?}, {{0x10582a409, 0x49}, {0x1058373c9, 0xb7}, ...})
	/Users/islam/dev/sourcenetwork/defradb/tests/integration/utils.go:85 +0x328
github.com/sourcenetwork/defradb/tests/integration/query/one_to_many.executeTestCase(...)
	/Users/islam/dev/sourcenetwork/defradb/tests/integration/query/one_to_many/utils.go:37
github.com/sourcenetwork/defradb/tests/integration/query/one_to_many.TestQueryFromSingleSideWithEqFilterOnRelatedType(0x14000509f98?)
	/Users/islam/dev/sourcenetwork/defradb/tests/integration/query/one_to_many/with_filter_related_id_test.go:334 +0x324
testing.tRunner(0x140007829c0, 0x105ee20c0)
	/Users/islam/sdk/go1.20.7/src/testing/testing.go:1576 +0x10c
created by testing.(*T).Run
	/Users/islam/sdk/go1.20.7/src/testing/testing.go:1629 +0x368
FAIL	github.com/sourcenetwork/defradb/tests/integration/query/one_to_many	0.665s
FAIL

To Reproduce

  • find TestQueryFromSingleSideWithEqFilterOnRelatedType test
  • replace this query
query {
	Author(filter: {published: {_key: {_eq: "bae-b9b83269-1f28-5c3b-ae75-3fb4c00d559d"}}}) {
		name
	}
}
  • with this logically equivalent one:
query {
	Author(filter: {
		_and: [ 
			{ name: { _ne: "Voltaire" }},
			{ published: {_key: {_eq: "bae-b9b83269-1f28-5c3b-ae75-3fb4c00d559d"}}}
		]
	}) {
		name
	}
}
  • run the test and observe the crash

Expected behavior
It should not panic and return the same result as the test with the original query.

Platform
mac

@islamaliev islamaliev added bug Something isn't working area/query Related to the query component labels Aug 23, 2023
@fredcarle
Copy link
Collaborator

Something is off in resolveInnerFilterDependencies.

The following will need to be changed (more as well but certainly that part).

if strings.HasPrefix(key, "_") && key != request.KeyFieldName {
  continue
}

islamaliev added a commit that referenced this issue Sep 11, 2023
## Relevant issue(s)

Resolves #1808 

## Description

Any time a filter with `_and` or `_or` operator that includes relations
is applied it will fail.
There were 2 problems:
- filter is not recursively analysed during mapping phase which would
result in some relational fields not being mapped and later panic upon
attempt to access them.
- the filter is not properly split between the scan and select nodes. It
would just analyse and split top-level fields.
These problems are fixed.
@AndrewSisley AndrewSisley added this to the DefraDB v0.7 milestone Sep 11, 2023
shahzadlone pushed a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
## Relevant issue(s)

Resolves sourcenetwork#1808 

## Description

Any time a filter with `_and` or `_or` operator that includes relations
is applied it will fail.
There were 2 problems:
- filter is not recursively analysed during mapping phase which would
result in some relational fields not being mapped and later panic upon
attempt to access them.
- the filter is not properly split between the scan and select nodes. It
would just analyse and split top-level fields.
These problems are fixed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/query Related to the query component bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants