Add function to check error on unique index. #74

pof3 · 2023-06-03T03:23:04Z

Just as the os module provides users with the os.IsNotExist function to test if the error was generated by trying to open a non existent file, regardless of its name, this commit adds the same functionality to check if the error comes from trying to insert data to a table with an unique index. This example, which is in the test generated for the function, shows its behavior:

...
err1 := errors.New("sending request failed for method 'create': There was a problem with the database: Database index `userNameIdx` already contains 'Mark', with record `user:⟨2⟩`")
err2 := errors.New("sending request failed for method 'create': There was a problem with the database: Database index \"''``\" already contains '\\', with record `user:⟨{ foo = []}⟩`")
err3 := errors.New("sending request failed for method 'create': There was a problem with the database: Database index `userNameIdx` THIS MUST NOT PASS, with record `user:⟨2⟩`")
err4 := errors.New(" Database index `uniqueBlob` already contains { date: 'today', location: 'London' }, with record `foo:2`")
err5 := errors.New(" database index `uniqueBlob` already contains { date: 'today', location: 'London' }, with record `foo:2`")
err6 := errors.New("Database record `foo:1` already exists")

fmt.Println(IsDuplicateUniqueIdx(err1))
fmt.Println(IsDuplicateUniqueIdx(err2))
fmt.Println(IsDuplicateUniqueIdx(err3))
fmt.Println(IsDuplicateUniqueIdx(err4))
fmt.Println(IsDuplicateUniqueIdx(err5))
fmt.Println(IsDuplicateUniqueIdx(err6))
...

Will print:

true
true
false
true
false
false

ElecTwix

Looks promising but it needs a rewrite for the PR's unit test.

db_test.go

pof3 · 2023-06-21T06:05:24Z

Just re wrote the tests using testify. all of them are passing.

ElecTwix · 2023-06-21T07:32:55Z

Just re wrote the tests using testify. all of them are passing.

you can resolve the discussions has been fixed with this commit.
Nice job. 👍

timpratim

The contribution is definitely helpful and looking forward to the final PR. The Todos are not for you to work on but for us to look into more. Happy to approve once the changes have been done.

-- Regex is known for decreasing performance and we will go ahead and add error code to make the implementation uniform across all drivers.

timpratim · 2023-07-17T12:02:59Z

db.go

+	if err == nil {
+		return false
+	}
+	match, _ := regexp.MatchString(`Database index .* already contains .*, with record .*`, err.Error())


ToDo:
Create ticket on SurrealDB project to include error code in error response.

timpratim · 2023-07-17T12:04:57Z

db_test.go

@@ -10,7 +10,7 @@ import (
 	"github.com/stretchr/testify/require"
 	"github.com/stretchr/testify/suite"
 	"github.com/surrealdb/surrealdb.go"
-	gorilla "github.com/surrealdb/surrealdb.go/pkg/gorilla"
+	"github.com/surrealdb/surrealdb.go/pkg/gorilla"


ToDo:

Add missing dependencies to depgaurd.

timpratim · 2023-07-17T12:06:17Z

db_test.go

@@ -145,7 +145,7 @@ func (s *SurrealDBTestSuite) TestCreate() {

 	s.Run("Single create works", func() {
 		userData, err := s.db.Create("users", testUser{
-			Username: "johnny",
+			Username: "tim",


Is this related to a test?
If not, please revert to original text.

timpratim · 2023-07-17T12:07:10Z

db_test.go

@@ -156,7 +156,7 @@ func (s *SurrealDBTestSuite) TestCreate() {
 		s.Require().NoError(err)
 		s.Len(userSlice, 1)

-		s.Equal("johnny", userSlice[0].Username)
+		s.Equal("tim", userSlice[0].Username)


timpratim · 2023-07-17T12:07:24Z

db_test.go

@@ -165,7 +165,7 @@ func (s *SurrealDBTestSuite) TestCreate() {
 		data := make([]testUser, 0)
 		data = append(data,
 			testUser{
-				Username: "johnny",
+				Username: "lu",


timpratim · 2023-07-17T12:08:34Z

db_test.go

+
+func (s *SurrealDBTestSuite) TestIsDuplicateUniqueIdx() {
+	query := `DEFINE INDEX uniqueNameIdx ON TABLE users COLUMNS username UNIQUE;`
+	_, err := s.db.Query(query, nil) // because of this query, I modified the 'username' field in other tests.


Tests should be independent. Please declare every table and data in this test.

timpratim · 2023-07-17T12:16:17Z

db_test.go

+	})
+	err = nil
+
+	// and finally, check with a tricky ID; this is the only way the function can give a false positive


Can we change this to an ignored testcase? That's because we want to verify important behaviour rather than verify implementation. Also it's also ok to have to a test in the comments rather than a working test.

pof3 · 2023-07-17T14:02:09Z

Thanks, @timpratim !
Yeah, I know regexp are not an ideal solution, in Go we'd rather use the built in functions errors.Is, errors.Unwrap, etc. Nonetheless, the current driver works on top of websockets, and no further modifications can be done without immediate effects on other drivers and stacks. As you say, returning the code error at the beginning would be a nice workaround, so we can standardize it and make the proper changes all together. This PR adds a quick way to do the job, so it might be helpful for users, and I think it wouldn't be an issue for backwards compatibility, since the underlying behaviour in future implementation will be the same as expected.

phughk · 2023-07-21T14:37:25Z

Linking this error ticket which also covers incorrect handling of error responses
#33
Agreed with @timpratim we are happy to go ahead with regex parsing 👍

Add function to check error on unique index.

07eebf1

ElecTwix suggested changes Jun 20, 2023

View reviewed changes

Write tests using testify.

1a4e906

timpratim reviewed Jul 17, 2023

View reviewed changes

timpratim mentioned this pull request Jul 17, 2023

Add dependencies to depguard #77

Open

timpratim added the nearly-ready label Jul 17, 2023

timpratim self-assigned this Jul 17, 2023

phughk mentioned this pull request Jul 21, 2023

Key path not found #33

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add function to check error on unique index. #74

Add function to check error on unique index. #74

pof3 commented Jun 3, 2023

ElecTwix left a comment

pof3 commented Jun 21, 2023

ElecTwix commented Jun 21, 2023 •

edited

Loading

timpratim left a comment

timpratim Jul 17, 2023

timpratim Jul 17, 2023

timpratim Jul 17, 2023

timpratim Jul 17, 2023

timpratim Jul 17, 2023

timpratim Jul 17, 2023

timpratim Jul 17, 2023

pof3 commented Jul 17, 2023

phughk commented Jul 21, 2023

Add function to check error on unique index. #74

Are you sure you want to change the base?

Add function to check error on unique index. #74

Conversation

pof3 commented Jun 3, 2023

ElecTwix left a comment

Choose a reason for hiding this comment

pof3 commented Jun 21, 2023

ElecTwix commented Jun 21, 2023 • edited Loading

timpratim left a comment

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

timpratim Jul 17, 2023

Choose a reason for hiding this comment

pof3 commented Jul 17, 2023

phughk commented Jul 21, 2023

ElecTwix commented Jun 21, 2023 •

edited

Loading