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.

Sign up for GitHub

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

Jump to bottom

interface conversion orderby panic #921

Closed
shahzadlone opened this issue Oct 28, 2022 · 0 comments · Fixed by #1094
Closed

interface conversion orderby panic #921

shahzadlone opened this issue Oct 28, 2022 · 0 comments · Fixed by #1094
Assignees
@shahzadlone
Labels
area/mapper Related to the mapper components area/testing Related to any test or testing suite bug Something isn't working
Milestone
DefraDB v0.5

Comments

@shahzadlone
Copy link
Member

Sub-task of #833

Description:

These tests located in tests/integration/query/one_to_many_to_one/with_sum_order_limit_test.go and tests/integration/query/one_to_many_to_one/with_order_limit_test.go file:

  • TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirections
  • TestOneToManyToOneDeepOrderBySubTypeOfBothDescAndAsc

Cause similar class of panics of interface conversion: interface {} is nil, not uint64.

Stack trace (of TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirections):

2022-10-28T05:29:45.729-0400, INFO, defra.tests.integration, 1-N-1 sum of deep orderby subtypes and non-sum deep orderby, asc. directions., {"Database": "badger-in-memory"}
panic: interface conversion: interface {} is nil, not uint64
        panic: Unclosed iterator at time of Txn.Discard. [recovered]
        panic: Unclosed iterator at time of Txn.Discard.

goroutine 11917 [running]:
testing.tRunner.func1.2({0x1154140, 0x1444fe0})
        /home/shahzad/.local/bin/go1.18.5/src/testing/testing.go:1389 +0x366
testing.tRunner.func1()
        /home/shahzad/.local/bin/go1.18.5/src/testing/testing.go:1392 +0x5d2
panic({0x1154140, 0x1444fe0})
        /home/shahzad/.local/bin/go1.18.5/src/runtime/panic.go:844 +0x258
github.com/dgraph-io/badger/v3.(*Txn).Discard(0xc0013a6000)
        /home/shahzad/go/pkg/mod/github.com/dgraph-io/badger/[email protected]/txn.go:523 +0x1f1
github.com/sourcenetwork/defradb/datastore/badger/v3.(*txn).discard(...)
        /home/shahzad/~~/defradb/datastore/badger/v3/datastore.go:915
github.com/sourcenetwork/defradb/datastore/badger/v3.(*txn).Discard(0xc000a20000, {0x4a1ff7?, 0x469938?})
        /home/shahzad/~~/defradb/datastore/badger/v3/datastore.go:911 +0xec
github.com/sourcenetwork/defradb/datastore.(*txn).Discard(0xc000266120, {0x144fa50, 0xc0001b6008})
        /home/shahzad/~~/defradb/datastore/txn.go:112 +0x58
panic({0x119db00, 0xc00144aba0})
        /home/shahzad/.local/bin/go1.18.5/src/runtime/panic.go:844 +0x258
github.com/sourcenetwork/defradb/db/base.Compare({0x1154380?, 0xc000bc7438?}, {0x0?, 0x0?})
        /home/shahzad/~~/defradb/db/base/compare.go:37 +0x85d
github.com/sourcenetwork/defradb/planner.(*valuesNode).docValueLess(0xc000a1da80, {0x0?, {0xc0005ee1b0?, 0xc000c2af58?, 0xc0001b6008?}}, {0xc0?, {0xc0060c8990?, 0x1099997?, 0xc0001bab40?}})
        /home/shahzad/~~/defradb/planner/values.go:105 +0x37d
github.com/sourcenetwork/defradb/planner.(*valuesNode).Less(0xc000a1da80, 0x1, 0x0)
        /home/shahzad/~~/defradb/planner/values.go:90 +0x197
sort.insertionSort({0x144e270, 0xc000a1da80}, 0x0, 0x5)
        /home/shahzad/.local/bin/go1.18.5/src/sort/sort.go:40 +0xd1
sort.quickSort({0x144e270, 0xc000a1da80}, 0x0, 0x5, 0x6)
        /home/shahzad/.local/bin/go1.18.5/src/sort/sort.go:222 +0x1d5
sort.Sort({0x144e270, 0xc000a1da80})
        /home/shahzad/.local/bin/go1.18.5/src/sort/sort.go:231 +0x65
github.com/sourcenetwork/defradb/planner.(*valuesNode).SortAll(...)
        /home/shahzad/~~/defradb/planner/values.go:80
github.com/sourcenetwork/defradb/planner.(*allSortStrategy).Finish(0xc000a541a8)
        /home/shahzad/~~/defradb/planner/order.go:206 +0x45
github.com/sourcenetwork/defradb/planner.(*orderNode).Next(0xc000266a20)
        /home/shahzad/~~/defradb/planner/order.go:147 +0x408
github.com/sourcenetwork/defradb/planner.(*limitNode).Next(0xc000c07a80)
        /home/shahzad/~~/defradb/planner/limit.go:67 +0xd8
github.com/sourcenetwork/defradb/planner.(*selectTopNode).Next(0xc000c0c820)
        /home/shahzad/~~/defradb/planner/select.go:70 +0x48
github.com/sourcenetwork/defradb/planner.(*typeJoinMany).Next(0xc000c34100)
        /home/shahzad/~~/defradb/planner/type_join.go:521 +0x530
github.com/sourcenetwork/defradb/planner.(*typeIndexJoin).Next(0xc000c32220)
        /home/shahzad/~~/defradb/planner/type_join.go:122 +0x48
github.com/sourcenetwork/defradb/planner.(*selectNode).Next(0xc0001ba000)
        /home/shahzad/~~/defradb/planner/select.go:143 +0x6c
github.com/sourcenetwork/defradb/planner.(*sumNode).Next(0xc000a32070)
        /home/shahzad/~~/defradb/planner/sum.go:191 +0x6f
github.com/sourcenetwork/defradb/planner.(*selectTopNode).Next(0xc000c0c870)
        /home/shahzad/~~/defradb/planner/select.go:70 +0x48
github.com/sourcenetwork/defradb/planner.(*Planner).executeRequest(0x0?, {0x112ed20?, 0xc000a03d40?}, {0x14540b8, 0xc000c0c870})
        /home/shahzad/~~/defradb/planner/planner.go:482 +0x83
github.com/sourcenetwork/defradb/planner.(*Planner).runRequest(0xc0008cc290?, {0x144fa50, 0xc0001b6008}, 0xc000a03d40)
        /home/shahzad/~~/defradb/planner/planner.go:527 +0x21d
github.com/sourcenetwork/defradb/planner.(*QueryExecutor).ExecQuery(0xc000af55c0?, {0x144fa50?, 0xc0001b6008}, {0x1456a68?, 0xc0000fdb80}, {0x1454d58?, 0xc000266120}, {0x12d1fa1, 0xed}, {0x0, ...})
        /home/shahzad/~~/defradb/planner/executor.go:79 +0x175
github.com/sourcenetwork/defradb/db.(*db).ExecQuery(0xc0000fdb80, {0x144fa50, 0xc0001b6008}, {0x12d1fa1, 0xed})
        /home/shahzad/~~/defradb/db/query.go:37 +0x33e
github.com/sourcenetwork/defradb/tests/integration.ExecuteQueryTestCase(0x19f7d40?, {0x12d24a3, 0x14c}, {0xc000c87d78, 0x3, 0x3}, {{0x12ccf81, 0x4d}, {0x12d1fa1, 0xed}, ...})
        /home/shahzad/~~/defradb/tests/integration/utils.go:396 +0x783
github.com/sourcenetwork/defradb/tests/integration/query/one_to_many_to_one.executeTestCase(...)
        /home/shahzad/~~/defradb/tests/integration/query/one_to_many_to_one/utils.go:49
github.com/sourcenetwork/defradb/tests/integration/query/one_to_many_to_one.TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirections(0x0?)
        /home/shahzad/~~/defradb/tests/integration/query/one_to_many_to_one/with_sum_order_limit_test.go:304 +0xe73
testing.tRunner(0xc0006f2680, 0x12f1870)
        /home/shahzad/.local/bin/go1.18.5/src/testing/testing.go:1439 +0x214
created by testing.(*T).Run
        /home/shahzad/.local/bin/go1.18.5/src/testing/testing.go:1486 +0x725
@shahzadlone shahzadlone added bug Something isn't working area/testing Related to any test or testing suite area/mapper Related to the mapper components labels Oct 28, 2022
@shahzadlone shahzadlone added this to the DefraDB v0.4 milestone Oct 28, 2022
@shahzadlone shahzadlone self-assigned this Oct 28, 2022
@shahzadlone shahzadlone mentioned this issue Oct 28, 2022
Closed
@shahzadlone shahzadlone modified the milestones: DefraDB v0.4, DefraDB v0.5 Dec 9, 2022
@shahzadlone shahzadlone mentioned this issue Feb 15, 2023
Merged
4 tasks
shahzadlone added a commit that referenced this issue Apr 13, 2023
@shahzadlone
fix: Make sort stable and handle nil comparison (#1094)
e140c81 
- Resolves #1071

- Resolves #921

- This change uses stable sort and ensures the `Less` interface implementation only returns true if the comparison is less, otherwise returns false (this is not the full story as for when we do `DESC` the less to us then is the check of if it's greater than instead of less than). Note: Ensures the ordering of the input array is preserved, so the output is always stable.

- This change works on both GoLang v1.18.5 and v1.19.5.

- This also resolves some nil panics we were having before (one subtask of #833, which is issue #921).
	1) Fixes the test: `TestOneToManyToOneDeepOrderBySubTypeOfBothDescAndAsc`
	2) Fixes the test: `TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirections`

- Ensures our sort handles `nil` sorting properly for `ASC` and `DESC`:
```
DESC = [10, 9, 8, ... nil, nil]
ASC  = [nil, nil, 1, 2, 3, ... ]
```
shahzadlone added a commit to shahzadlone/defradb that referenced this issue Feb 23, 2024
@shahzadlone
fix: Make sort stable and handle nil comparison (sourcenetwork#1094)
16d3f17 
- Resolves sourcenetwork#1071

- Resolves sourcenetwork#921

- This change uses stable sort and ensures the `Less` interface implementation only returns true if the comparison is less, otherwise returns false (this is not the full story as for when we do `DESC` the less to us then is the check of if it's greater than instead of less than). Note: Ensures the ordering of the input array is preserved, so the output is always stable.

- This change works on both GoLang v1.18.5 and v1.19.5.

- This also resolves some nil panics we were having before (one subtask of sourcenetwork#833, which is issue sourcenetwork#921).
	1) Fixes the test: `TestOneToManyToOneDeepOrderBySubTypeOfBothDescAndAsc`
	2) Fixes the test: `TestOneToManyToOneWithSumOfDeepOrderBySubTypeAndDeepOrderBySubtypeAscDirections`

- Ensures our sort handles `nil` sorting properly for `ASC` and `DESC`:
```
DESC = [10, 9, 8, ... nil, nil]
ASC  = [nil, nil, 1, 2, 3, ... ]
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@shahzadlone shahzadlone

Labels
area/mapper Related to the mapper components area/testing Related to any test or testing suite bug Something isn't working
Projects
None yet
Milestone
DefraDB v0.5
Development

Successfully merging a pull request may close this issue.

fix: Make sort stable and handle nil comparison sourcenetwork/defradb
1 participant
@shahzadlone