Comments (10)
If we want to use generics (locking us into go1.18+) we could have a cleaner solution something like this
type Response[T any] struct {
Time string `json:"time"`
Status string `json:"status"`
Result []T `json:"result"`
}
and then the functions like Select
just directly unmarshal it using that data type
from surrealdb.go.
This is a good option, but sadly as of go1.19 we can't have generic methods (only on the types over which the receiver is generic) so the function returning the Result would need to be top-level like
surrealdb.Select[User](ctx, db, "* from user")
and I find that less useful and organic than a simple db.Select(ctx, "* from user")
Sadly i don't know any library using generics like that today so i don't know how viable this is as an API
Maybe this could exist as a replacement for the top-level Unmarshal ?
We return some kind of raw result (stored as []byte ?) that has some basic methods, and a top-level UnmarshalAs[T any](raw)
This is a bit more cumbersome though
from surrealdb.go.
My original branch I forked from #3 converted the Unmarshal (and RawUnmarshal that I added) to use generics so I could have syntax similar to the code in @Keitio's comment above, but I switched to any
for compatibility reasons. I think compatibility should probably be prioritized for a project like this, as it would help with adoption of Surreal.
I am currently working on the issue of multiple marshals / unmarshals mentioned in #17 and may be able to add a way to do this with any
while I'm changing everything around. I am currently returning []byte from everything, so I just need to modify each of the functions to add an additional any
parameter and call the updated unmarshal functions that I am working on. I can make an additional comment with the branch when I finish in a few hours.
from surrealdb.go.
If we want compatability then we should do interface{} i think. any is a go1.18 generic thing and is just an alias
from surrealdb.go.
This isn't really a great implementation, but I got something that works:
https://github.com/garrison-henkle/surrealdb.go/tree/fix_unmarshaling_performance
The functions now work like this:
type testUser struct {
ID string `json:"id"`
Name string `json:"name"`
}
user := testUser{
Name: "garrison",
}
var response testUser
ok, err := db.Create("users", &user, &response)
if err != nil{
panic(err)
}
if !ok{
panic("empty result from database")
}
fmt.Println(response.Name) //prints "garrison"
I added an ok
boolean return to all of the database methods to address no result responses, so it is pretty clunky to use due to double checks after every call. It also uses really simple means of parsing the json strings, such as hard coded indices and looking for certain characters. It's better than marshalling then unmarshalling, but would likely need to be changed to something more robust for something like this to be merged in. I also didn't add any safe guards, so the users would need to know to use a slice if more than one result is possible. Generics provides an easy fix for the first two issues honestly, so it might be nice to have two separate forms of the driver: one with clunky interface{}
functions and one with generics.
If we want compatability then we should do interface{} i think. any is a go1.18 generic thing and is just an alias
Sorry, I meant interface{}
instead of any
. I probably should not have used them interchangably in a comment explicitly talking about backwards compatibility 😆. I've been using interface{}
in my changes to this repo since one of the early PRs that swapped out all the any
s.
from surrealdb.go.
for the (ok, err)
problem, you should just return an error, with some ErrNoResult
we could check against, like we do for io.EOF
But we need to have better error handling in most places anyway
Using another parameter to unmarshal into is nice, but locks us into non-generics sadly, and generics migration would be breaking or clunky
For now, i still think that the raw result return, as strange as it is, is the more flexible option as it would allow both generic and non generic behaviors
MongoDB's driver does something similar for some calls so it's not entirely unheard of.
from surrealdb.go.
So, since this ties into performance partially, I'm going to make a new issue and show an idea i had, which also ties into a way to avoid the interface{}/any responses and such, we can all discuss the performance/json unmarshalling side there
from surrealdb.go.
@Keitio mentioned Mongo, so I tried to clean up my changes and mimic a bit of its driver code to see what it would look like. I ended up with something like this:
var users []testUser
//normal method
ok, err = db.Select("testUser").Unmarshal(&users)
//query
ok, err = db.Query("select * from testUser", nil).UnmarshalRaw(&users)
I added two structs that return from the send
function in db.go
to encapsulate the data better and allow for the unmarshal functions to be methods:
//returned by normal methods
type SurrealWSResult struct {
Result []byte
Single bool
Error error
}
//returned by query
type SurrealWSRawResult struct {
Result []byte
Error error
}
I actually like this a lot better than the previous attempts I've made because it inlines the unmarshaling. It also prevents use of the wrong unmarshal function (ie using the raw function for Select
or the normal one for Query
).
The branch with all my changes is here if anyone wants to browse through them or suggest changes.
for the (ok, err) problem, you should just return an error, with some ErrNoResult we could check against, like we do for io.EOF
Maybe this is because I don't have much experience in Go (mostly JVM background), but should a no result response really be an error? If I send a raw query using Query
that returns no results, the status attached to that no result response will be "status"="OK"
. It just feels wrong to me to treat a successful response as an error, but, again, I don't know enough about Go style to know any better.
from surrealdb.go.
Basically for empty result, it depends.
For a SelectAll
kind of method, we should just return an empty error
For a SelectOne
that is empty, we should return an error
But either way, returning a bool and an error is bad practice, because it should be an error (on select 1) or a non-event (on select all). If you want to check for empty result, you can just directly do if len(result) == 0 {...}
I'm all for this API though, I would just rename UnmarshalRaw
to just Unmarshal
, to be more consistent with the other Result type. Maybe the internal implementation could be a bit different bu i like the API
from surrealdb.go.
Yeah that's fair. I can switch the bool out for an error and make that signature change when I'm cleaning up the code later.
from surrealdb.go.
Related Issues (20)
- Feature: context.Context Support HOT 6
- Documentation: working with record links HOT 4
- Feature: Move and test `unmarshalMapToStruct(data map[string]interface{}, outStruct interface{})` HOT 2
- Bug: Re-connection to the DB HOT 2
- Bug: Broken Tests for Fetch HOT 5
- Bug: Unhandled errors in websockets can lead to panics HOT 4
- Bug: `Question or idea` Button HOT 2
- surrealdb Auth undefined HOT 1
- Bug: unexpected EOF on TestSurrealDBSuite/gorilla/TestUpdate HOT 1
- Documentation: The README file example is not usable with the latest main branch, neither with the latest version HOT 1
- Feature: Replace Gorilla WS with Nhooyr WS HOT 6
- Bug: The session has expired error when connecting to version 1.3.0 HOT 4
- Bug: stack overflow error in gorilla websocket with db version 1.3.0 HOT 4
- Feature: surrealML for golang
- Documentation: Add Documentation for .Live() func & other stuff related to LIVE queries HOT 4
- Add native support for batch inserting data HOT 2
- Bug: Cannot use type::thing in queries as they arent escaped properly HOT 2
- Feature: better error return HOT 9
- Feature: Formatting Query like fmt.Sprintf HOT 2
- Example fails in Beta 11 - panic: sending request failed for method 'change': Method not found HOT 2
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from surrealdb.go.