Return error instead of panic when geo data is corrupted

[mangalaman93]

Fixes #3740

We can still add corrupted data into the database. I think, there should be separate effort to fix that. Adding corrupt data is possible for other types as well. This PR ensures that alpha doesn't crash when corrupt data is encountered while querying.

---

This change is 

[poonai]

Reviewed 1 of 2 files at r1.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @mangalaman93 and @pawanrawal)

---

query/query.go, line 399 at r1 (raw file):

	if err != nil {
		// This can happen when a mutation ingests corrupt data into the database.
		return v, errors.Wrap(err, "Error while interpreting appropriate type from binary")

Is it possible to show in error message that we failed to convert into this type by inferring the typeID.

[martinmr]

Just a small comment but otherwise it

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mangalaman93 and @pawanrawal)

---

query/query3_test.go, line 2259 at r1 (raw file):

}`
	_, err = dg.NewReadOnlyTxn().Query(ctx, query)
	require.Contains(t, err.Error(), "wkb: unknown byte order: 1111011")

So why does this fail? Maybe add a comment.

Also, if you can't query the data, should the mutate be allowed to succeed?

[pawanrawal]

Reviewed 2 of 2 files at r1.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @balajijinnah and @mangalaman93)

---

query/query.go, line 399 at r1 (raw file):

Previously, balajijinnah (balaji) wrote…

Is it possible to show in error message that we failed to convert into this type by inferring the typeID.

We should add the attr and the type name to the error message. Also would be good to have an error log for this since this is a serious enough issue.

---

query/query3_test.go, line 2259 at r1 (raw file):

}`
	_, err = dg.NewReadOnlyTxn().Query(ctx, query)
	require.Contains(t, err.Error(), "wkb: unknown byte order: 1111011")

Are we messing up the byte order while storing the data? Little/big endian somewhere?

[mangalaman93]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @balajijinnah, @martinmr, and @pawanrawal)

---

query/query.go, line 399 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

We should add the attr and the type name to the error message. Also would be good to have an error log for this since this is a serious enough issue.

Added attr to error information

---

query/query3_test.go, line 2259 at r1 (raw file):

Previously, martinmr (Martin Martinez Rivera) wrote…

So why does this fail? Maybe add a comment.

Also, if you can't query the data, should the mutate be allowed to succeed?

Done.

---

query/query3_test.go, line 2259 at r1 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Are we messing up the byte order while storing the data? Little/big endian somewhere?

Nope, we are storing string json data in geo value which should be marshalled using wkb.Marshal function. This example shows how we could potentially use GeoVal API but probably shouldn't.

[mangalaman93]

Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @balajijinnah, @martinmr, and @pawanrawal)

---

query/query3_test.go, line 2259 at r1 (raw file):

Previously, mangalaman93 (Aman Mangal) wrote…

Done.

Ideally, mutation shouldn't success but this will happen for all the data types. Adding a check in mutation will probably slow down the mutation. It makes sense to look into it though.

[manishrjain]

Reviewed 2 of 2 files at r2.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @balajijinnah, @martinmr, and @pawanrawal)

[pawanrawal]

Reviewed 1 of 2 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @balajijinnah, @martinmr, and @pawanrawal)