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

Emulator does not infer DATE & TIMESTAMP type for null values #31

Closed
larkee opened this issue Jun 23, 2021 · 7 comments
Closed

Emulator does not infer DATE & TIMESTAMP type for null values #31

larkee opened this issue Jun 23, 2021 · 7 comments
Assignees
Labels
enhancement New feature or request

Comments

@larkee
Copy link

larkee commented Jun 23, 2021

Originally raised in googleapis/python-spanner-django#566, Spanner will infer the type of a null value if it is not provided but the emulator will throw an error for the same request. This has become an issue for Python ORMs as the suggestion to add this type inference logic to the DBAPI adapter is complicated and requires many changes.

Adding type inference for null values to the emulator to match the production behaviour would resolve the problem and allow us to confidently test against the emulator again.

@larkee larkee added the enhancement New feature or request label Jun 23, 2021
olavloite added a commit to googleapis/google-cloud-go that referenced this issue Jul 22, 2021
Skip testing DML with untyped parameters on the emulator as it does
not support untyped parameters.

See GoogleCloudPlatform/cloud-spanner-emulator#31
olavloite added a commit to googleapis/google-cloud-go that referenced this issue Jul 22, 2021
* feat(spanner): Allow untyped nil values in params

Removes the restriction that nil valued parameters must be typed. The current
restriction prevents the usage of DML statements with one or more untyped nil
parameter values to be used to insert/update a value to NULL. Cloud Spanner
does not require all parameters to be typed, only when it would otherwise be
ambigous what type should be used (e.g. non-NULL STRING/BYTES values).

This restriction also prevents the usage of the go/sql driver with gorm, as
gorm will generate statements with untyped nil parameter values.

Fixes #4481

* fix: skip DML test on emulator

Skip testing DML with untyped parameters on the emulator as it does
not support untyped parameters.

See GoogleCloudPlatform/cloud-spanner-emulator#31

* fix: add missing !
@snehashah16
Copy link
Collaborator

Hello @larkee @olavloite

I was trying to debug this issue and would like some steps to reproduce this issue. I tried the following tests and have got a successful run from the emulator. Can u please provide the test case, logs ( --log_requests CLI argument to the emulator) and the error message.

Python Test:

  1. Parameterized DML with some types defined: Passes
update_statement = (
            "UPDATE contacts SET email = @email " "WHERE contact_id = @contact_id;",
            {"contact_id": 1, "email": None},
            {"contact_id": Type(code=INT64),},
        )

Go Test: PASSES, except for DATES, TIMESTAMP

	tests := []struct {
		col  string
		val  interface{}
		want interface{}
	}{
		{col: "String", val: nil, want: NullString{}},
		{col: "StringArray", val: nil, want: []NullString(nil)},
		{col: "Bytes", val: nil, want: []byte(nil)},
		{col: "BytesArray", val: nil, want: [][]byte(nil)},
		{col: "Int64a", val: nil, want: NullInt64{}},
		{col: "Int64Array", val: nil, want: []NullInt64(nil)},
		{col: "Bool", val: nil, want: NullBool{}},
		{col: "BoolArray", val: nil, want: []NullBool(nil)},
		{col: "Float64", val: nil, want: NullFloat64{}},
		{col: "Float64Array", val: nil, want: []NullFloat64(nil)},
		// FAILS	{col: "Date", val: nil, want: NullDate{}},
		// FAILS	{col: "DateArray", val: nil, want: []NullDate(nil)},
		// FAILS	{col: "Timestamp", val: nil, want: NullTime{}},
		// FAILS	{col: "TimestampArray", val: nil, want: []NullTime(nil)},
	}
        statements := make([]Statement, 0)
	for i, test := range tests {
		stmt := NewStatement(fmt.Sprintf("INSERT INTO Types (RowId, `%s`) VALUES (@id, @value)", test.col))
		// Note: We are not setting the parameter type here to ensure that it
		// can be automatically recognized when it is actually needed.
		stmt.Params["id"] = i
		stmt.Params["value"] = test.val
		log.Println("Statement: %v", stmt)
		statements = append(statements, stmt)
	}
	_, err := client.ReadWriteTransaction(ctx, func(ctx context.Context, tx *ReadWriteTransaction) error {
		rowCounts, err := tx.BatchUpdate(ctx, statements)
        })

@snehashah16 snehashah16 self-assigned this Feb 3, 2022
@olavloite
Copy link
Contributor

@snehashah16 I think your conclusion that this only fails for DATE and TIMESTAMP is correct, and that the initial issue report missed that, simply because the example statements included these types and it was wrongly assumed that it applied to all types. I can confirm that untyped null parameter values work on the emulator for all types (including NUMERIC and JSON), except for DATE and TIMESTAMP.

Interestingly enough that also means that ARRAY<TIMESTAMP> and ARRAY<DATE> also work with untyped null values, so it is really only the base types DATE and TIMESTAMP that fail.

@snehashah16
Copy link
Collaborator

Thanks for confirming @olavloite .. Could we please uncomment the Golang tests for the untyped parameters for the emulator test run.

I will keep this bug open for DATE and TIMESTAMP.

@snehashah16 snehashah16 changed the title Emulator does not infer type for null values Emulator does not infer DATE & TIMESTAMP type for null values Feb 3, 2022
olavloite added a commit to googleapis/google-cloud-go that referenced this issue Feb 3, 2022
The emulator now supports all data types that are also supported by
Cloud Spanner. The integration tests also unnecessarily skipped a large
number of tests that tried to insert untyped null values as it was
assumed that this was not supported. That is however supported for most
data types, and only DATE and TIMESTAMP need to be skipped.

Updates #GoogleCloudPlatform/cloud-spanner-emulator#31
olavloite added a commit to googleapis/google-cloud-go that referenced this issue Feb 4, 2022
* test(spanner): enable more tests on emulator

The emulator now supports all data types that are also supported by
Cloud Spanner. The integration tests also unnecessarily skipped a large
number of tests that tried to insert untyped null values as it was
assumed that this was not supported. That is however supported for most
data types, and only DATE and TIMESTAMP need to be skipped.

Updates #GoogleCloudPlatform/cloud-spanner-emulator#31

* fix: remove unused code

* fix: use latest emulator version

* fix: bump emulator version to 1.4.1
@jakehilton
Copy link

I've just hit this issue as well. Seems less of an enhancement request and more of a straight up bug. Curious if any progress has been made on this front. Here's to hoping! Thanks!

@mohitg1580
Copy link

@jakehilton - we are looking into this issue, and will give you an ETA as soon as we have it.

@gauravpurohit06
Copy link
Collaborator

We have made the bug fixes related to this issue in the latest release. Please verify.

olavloite added a commit to googleapis/java-spanner that referenced this issue Apr 18, 2023
Add tests to verify that untyped null statement parameters work with
both GoogleSQL, PostgreSQL and the emulator.

Verifies that GoogleCloudPlatform/cloud-spanner-emulator#31
has been fixed.
@olavloite
Copy link
Contributor

The tests in googleapis/java-spanner#2388 verify that this issue has now been fixed in 1.5.3 of the emulator.

olavloite added a commit to googleapis/java-spanner that referenced this issue Apr 21, 2023
Add tests to verify that untyped null statement parameters work with
both GoogleSQL, PostgreSQL and the emulator.

Verifies that GoogleCloudPlatform/cloud-spanner-emulator#31
has been fixed.
jschaf pushed a commit to jschaf/spanner-go that referenced this issue Oct 10, 2024
* feat(spanner): Allow untyped nil values in params

Removes the restriction that nil valued parameters must be typed. The current
restriction prevents the usage of DML statements with one or more untyped nil
parameter values to be used to insert/update a value to NULL. Cloud Spanner
does not require all parameters to be typed, only when it would otherwise be
ambigous what type should be used (e.g. non-NULL STRING/BYTES values).

This restriction also prevents the usage of the go/sql driver with gorm, as
gorm will generate statements with untyped nil parameter values.

Fixes #4481

* fix: skip DML test on emulator

Skip testing DML with untyped parameters on the emulator as it does
not support untyped parameters.

See GoogleCloudPlatform/cloud-spanner-emulator#31

* fix: add missing !
jschaf pushed a commit to jschaf/spanner-go that referenced this issue Oct 10, 2024
* test(spanner): enable more tests on emulator

The emulator now supports all data types that are also supported by
Cloud Spanner. The integration tests also unnecessarily skipped a large
number of tests that tried to insert untyped null values as it was
assumed that this was not supported. That is however supported for most
data types, and only DATE and TIMESTAMP need to be skipped.

Updates #GoogleCloudPlatform/cloud-spanner-emulator#31

* fix: remove unused code

* fix: use latest emulator version

* fix: bump emulator version to 1.4.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

7 participants