Allow custom types as indexes when joining tables

[emarj]

See #198

The change is quite simple. We check if the field is tagged as primary key, if so, we add it as an index regardless of its type.

[codecov-commenter]

Codecov Report

Merging #197 (31cddca) into master (4873e43) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #197      +/-   ##
==========================================
- Coverage   92.76%   92.76%   -0.01%     
==========================================
  Files         116      116              
  Lines        6929     6927       -2     
==========================================
- Hits         6428     6426       -2     
  Misses        400      400              
  Partials      101      101              

Impacted Files	Coverage Δ
qrm/scan_context.go	94.82% <100.00%> (-0.06%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[go-jet]

Good catch. 💯 Could you also include one test in ./tests/postgres (or mysql).

[go-jet]

Maybe we can reorder this condition, to avoid two ifs with negations.

if isPrimaryKey(...) {
...
} else !isSimpleModelType(fieldType) {
...
}

[go-jet]

Actually it can be just:

if isPrimaryKey(...) {
...
} else fieldType.Kind() == reflect.Struct {
...
}

[emarj]

Yes you are right. Done!

[emarj]

I added the test. Unfortunately I was not able to properly configure the test environment on my local machine. I hope it passes on CI.

[go-jet]

It seems test fails because json doesn't match. Expected json:

[
	{
		"CityID": {
			"Int64": 312,
			"Valid": true
		},
		"CityName": "London",
		"Customers": [
			{
				"CustomerID": {
					"Int64": 252,
					"Valid": true
				},
				"LastName": "Hoffman",
				"Address": {
					"AddressID": {
						"Int64": 256,
						"Valid": true
					},
					"AddressLine": "1497 Yuzhou Drive"
				}
			},
			{
				"CustomerID": {
					"Int64": 512,
					"Valid": true
				},
				"LastName": "Vines",
				"Address": {
					"AddressID": {
						"Int64": 517,
						"Valid": true
					},
					"AddressLine": "548 Uruapan Street"
				}
			}
		]
	},
	{
		"CityID": {
			"Int64": 589,
			"Valid": true
		},
		"CityName": "York",
		"Customers": [
			{
				"CustomerID": {
					"Int64": 497,
					"Valid": true
				},
				"LastName": "Sledge",
				"Address": {
					"AddressID": {
						"Int64": 502,
						"Valid": true
					},
					"AddressLine": "1515 Korla Way"
				}
			}
		]
	}
]

[emarj]

Yes, it was silly on my part. I am so used to null.Int by github.com/guregu/null that I forgot about the JSON marshaling of sql.NullInt64. I also added another test by using the aforementioned null.Int.

I was able to configure the environment to run some test, even tough not everything works. I had to pass trough some hoops to make it work on my M1 Mac. In my case platform: linux/amd64 was needed in docker-compose.yaml.

[go-jet]

Yeah, I've heard M1 Mac tends to have problems with docker. Maybe we can include your docker-compose.yaml changes. M1 is getting popular, someone else might stumble on the same issue. It can be a separate PR.

[go-jet]

Perfect. LGTM.