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

Missing tests for join and inline array access from within a groupby #391

Closed
AndrewSisley opened this issue May 3, 2022 · 3 comments · Fixed by #752
Closed

Missing tests for join and inline array access from within a groupby #391

AndrewSisley opened this issue May 3, 2022 · 3 comments · Fixed by #752
Assignees
Labels
area/query Related to the query component code quality Related to improving code quality documentation Improvements or additions to documentation

Comments

@AndrewSisley
Copy link
Contributor

AndrewSisley commented May 3, 2022

For example the below is not tested:

query {
    author (groupBy: [age]) {
        age
        C1: _count(_group:{})
        C2: _count(published:{})
        _group {
            name
        }
        published {
            name
            rating
        }
    }
}
@AndrewSisley AndrewSisley added documentation Improvements or additions to documentation code quality Related to improving code quality labels May 3, 2022
@AndrewSisley AndrewSisley changed the title Aggregate: Missing tests for join and inline array access from within a groupby Missing tests for join and inline array access from within a groupby May 3, 2022
@jsimnz jsimnz added this to the DefraDB v0.3.1 milestone May 6, 2022
@AndrewSisley AndrewSisley self-assigned this Jun 21, 2022
@AndrewSisley AndrewSisley added the bug Something isn't working label Jun 21, 2022
@AndrewSisley
Copy link
Contributor Author

There is a bug in the join code here, below test fails (does not return book "Histoiare des Celtes..."):

func TestQueryOneToManyWithParentJoinGroupNumberAndUnrenderedGroup(t *testing.T) {
	test := testUtils.QueryTestCase{
		Description: "One-to-many relation query from many side with parent level group, unrendered group",
		Query: `query {
			author (groupBy: [age]) {
				age
				published {
					name
					rating
				}
			}
		}`,
		Docs: map[int][]string{
			//books
			0: { // bae-fd541c25-229e-5280-b44b-e5c2af3e374d
				`{
					"name": "Painted House",
					"rating": 4.9,
					"author_id": "bae-41598f0c-19bc-5da6-813b-e80f14a10df3"
				}`,
				`{
					"name": "A Time for Mercy",
					"rating": 4.5,
					"author_id": "bae-41598f0c-19bc-5da6-813b-e80f14a10df3"
					}`,
				`{
					"name": "The Client",
					"rating": 4.5,
					"author_id": "bae-41598f0c-19bc-5da6-813b-e80f14a10df3"
					}`,
				`{
					"name": "Candide",
					"rating": 4.95,
					"author_id": "bae-7accaba8-ea9d-54b1-92f4-4a7ac5de88b3"
				}`,
				`{
					"name": "Zadig",
					"rating": 4.91,
					"author_id": "bae-7accaba8-ea9d-54b1-92f4-4a7ac5de88b3"
				}`,
				`{
					"name": "Histoiare des Celtes et particulierement des Gaulois et des Germains depuis les temps fabuleux jusqua la prise de Roze par les Gaulois",
					"rating": 2,
					"author_id": "bae-09d33399-197a-5b98-b135-4398f2b6de4c"
				}`,
			},
			//authors
			1: {
				// bae-41598f0c-19bc-5da6-813b-e80f14a10df3
				`{
					"name": "John Grisham",
					"age": 65,
					"verified": true
				}`,
				// bae-7accaba8-ea9d-54b1-92f4-4a7ac5de88b3
				`{
					"name": "Voltaire",
					"age": 327,
					"verified": true
				}`,
				// bae-09d33399-197a-5b98-b135-4398f2b6de4c
				`{
					"name": "Simon Pelloutier",
					"age": 327,
					"verified": true
				}`,
			},
		},
		Results: []map[string]interface{}{
			{
				"age": uint64(327),
				"published": []map[string]interface{}{
					{
						"name":   "Histoiare des Celtes et particulierement des Gaulois et des Germains depuis les temps fabuleux jusqua la prise de Roze par les Gaulois",
						"rating": float64(2),
					},
					{
						"name":   "Candide",
						"rating": 4.95,
					},
					{
						"name":   "Zadig",
						"rating": 4.91,
					},
				},
			},
			{
				"age": uint64(65),
				"published": []map[string]interface{}{
					{
						"name":   "The Client",
						"rating": 4.5,
					},
					{
						"name":   "Painted House",
						"rating": 4.9,
					},
					{
						"name":   "A Time for Mercy",
						"rating": 4.5,
					},
				},
			},
		},
	}

	executeTestCase(t, test)
}

@AndrewSisley
Copy link
Contributor Author

There is a branch (sisley/test/I391-group-by-join-tests) with some of the first tests for this, but for now this is blocked by #471 as I don't want to keep stacking prod code changes on top of the pending PR.

@AndrewSisley AndrewSisley added the area/query Related to the query component label Jun 21, 2022
@AndrewSisley
Copy link
Contributor Author

I was confused when I reported the bug - it is behaving correctly "Histoiare des Celtes..." should not show up unless within a _group {...}

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 code quality Related to improving code quality documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants