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

Type encoding #761

Merged
merged 3 commits into from
May 24, 2024
Merged

Type encoding #761

merged 3 commits into from
May 24, 2024

Conversation

brennanjl
Copy link
Collaborator

@brennanjl brennanjl commented May 22, 2024

This commit adds integration tests for the new data types supported in Kwil v0.8.
In order to achieve this, this PR makes several changes to how data is encoded and handled within Kwil.

Transactions

Data sent via transaction must be RLP encoded into a new struct that specifies the type
that is encoded. The backend will still attempt to coerce types into the appropriate type,
but it can not always do this. In particular, BLOBs and any array type must be explicitly encoded.
Furthermore, the way in which nils are sent has changed, as they can now be encoded as a nil type.

Postgres

Kwil will now decode values from Postgres into their respective Kwil native type. The exception
is for uint256s, which get decoded as numerics. This is due to Kwil's underlying uint256 domain
being based off of Postgres's numeric type. When using inferred type mode (which is only used for
in-line actions expressions), Kwil will also attempt to coerce the incoming argument types. In general,
end-users should still be able to send everything as a string, unless they are doing math within
an action's in-line expression.

CLI

This also makes a minor breaking change to the CLI. Previously, the CLI was unable to encode nil values.
Now, if a value is not specified, it will be encoded as nil.

It also adds support for sending blobs via the CLI. In order to do this, users must base64 encode the blobs,
and add ;b64 to the end. The CLI will attempt to parse all inputs that end with this as blobs.

Finally, this PR also adds support for using arrays in the CLI. Users simply need to delimit their array values
by commas, and the CLI will parse it as such. If a user wants to use a comma without making it an array, they
can simply wrap the string in either double or single quotes.

return json.Marshal(u.String())
}

func (u UUID) UnmarshalJSON(b []byte) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unmarshal/Scan type methods should almost always be pointer receiver on the other hand.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

func Test_UUIDJSON(t *testing.T) {
	seed := []byte("test")

	uuid := types.NewUUIDV5(seed)

	b, err := json.Marshal(uuid)
	if err != nil {
		t.Fatal(err)
	}

	t.Log(uuid)

	var uuid2 types.UUID
	err = json.Unmarshal(b, uuid2) // call of Unmarshal passes non-pointer as second argumentunmarshaldefault
	if err != nil {
		t.Error(err)
	}

	var uuid3 types.UUID
	err = json.Unmarshal(b, &uuid3)
	if err != nil {
		t.Fatal(err)
	}
	t.Log(uuid3) // 00000000-0000-0000-0000-000000000000
}
=== RUN   Test_UUIDJSON
    /home/jon/kwil/git/kwil-db/core/types/uuid_test.go:30: 24aa70cf-0e18-57c9-b449-da8c9db37821
    /home/jon/kwil/git/kwil-db/core/types/uuid_test.go:35: json: Unmarshal(non-pointer types.UUID)
    /home/jon/kwil/git/kwil-db/core/types/uuid_test.go:43: 00000000-0000-0000-0000-000000000000
--- FAIL: Test_UUIDJSON (0.00s)
FAIL
coverage: 2.8% of statements
FAIL	github.com/kwilteam/kwil-db/core/types	0.013s

core/types/uuid.go Outdated Show resolved Hide resolved
Comment on lines 54 to 56
// Over json, we want to send uuids as strings
func (u *UUID) MarshalJSON() ([]byte, error) {
return json.Marshal(u.String())
Copy link
Member

@jchappelow jchappelow May 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some trouble here now. You get different results if you marshal a value vs a pointer. Here's where a value receiver for the marshal method is normally the way:

func Test_UUIDJSON(t *testing.T) {
	seed := []byte("test")

	uuidPtr := types.NewUUIDV5(seed)
	uuidVal := *uuidPtr

	b, err := json.Marshal(uuidPtr)
	if err != nil {
		t.Fatal(err)
	}

	t.Log(string(b)) // "24aa70cf-0e18-57c9-b449-da8c9db37821"

	var uuid2 types.UUID
	err = json.Unmarshal(b, &uuid2)
	if err != nil {
		t.Fatal(err)
	}
	t.Log(uuid2) // 24aa70cf-0e18-57c9-b449-da8c9db37821

	b, err = json.Marshal(uuidVal)
	if err != nil {
		t.Fatal(err)
	}

	t.Log(string(b)) // [36,170,112,207,14,24,87,201,180,73,218,140,157,179,120,33] (oops, didn't use MarshalJSON method!)

	var uuid3 types.UUID
	err = json.Unmarshal(b, &uuid3)
	if err != nil {
		t.Fatal(err)  // json: cannot unmarshal array into Go value of type string
	}
	t.Log(uuid3)
}

…n Kwil v0.8.

In order to achieve this, this PR makes several changes to how data is encoded and handled within Kwil.

Data sent via transaction must be RLP encoded into a new struct that specifies the type
that is encoded. The backend will still attempt to coerce types into the appropriate type,
but it can not always do this. In particular, BLOBs and any array type must be explicitly encoded.
Furthermore, the way in which nils are sent has changed, as they can now be encoded as a nil type.

Kwil will now decode values from Postgres into their respective Kwil native type. The exception
is for uint256s, which get decoded as numerics. This is due to Kwil's underlying uint256 domain
being based off of Postgres's numeric type. When using inferred type mode (which is only used for
in-line actions expressions), Kwil will also attempt to coerce the incoming argument types. In general,
end-users should still be able to send everything as a string, _unless_ they are doing math within
an action's in-line expression.

This also makes a minor breaking change to the CLI. Previously, the CLI was unable to encode nil values.
Now, if a value is not specified, it will be encoded as nil.

It also adds support for sending blobs via the CLI. In order to do this, users must base64 encode the blobs,
and add `;b64` to the end. The CLI will attempt to parse all inputs that end with this as blobs.

Finally, this PR also adds support for using arrays in the CLI. Users simply need to delimit their array values
by commas, and the CLI will parse it as such. If a user wants to use a comma without making it an array, they
can simply wrap the string in either double or single quotes.
Copy link
Contributor

@Yaiba Yaiba left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did a quick scan and lgtm


val, ok := input[inputField]
if !ok {
fmt.Println(len(newTuple))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this output needed? Feels like it's for debug

@brennanjl brennanjl merged commit b646ff5 into main May 24, 2024
2 checks passed
@brennanjl brennanjl deleted the type-encoding branch May 24, 2024 19:22
@Yaiba Yaiba added this to the v0.8.0 milestone May 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants