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

untyped prop diff in old vs new table sdk. #24109

Closed
paulgmiller opened this issue Feb 12, 2025 · 3 comments · Fixed by #24119
Closed

untyped prop diff in old vs new table sdk. #24109

paulgmiller opened this issue Feb 12, 2025 · 3 comments · Fixed by #24119
Labels
Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Tables

Comments

@paulgmiller
Copy link
Member

Bug Report

Incompatiblity between old github.com/Azure/azure-sdk-for-go/[email protected] and new aztables. (github.com/Azure/azure-sdk-for-go/sdk/data/aztables v1.3.0)

The new version tries to convert an untyped value in aztables.EDMEntity to int32
blob/dc2f5075a50d6e2cbe41b7787fe6e3d8b5eac00f/sdk/data/aztables/entity.go#L101
the old sdk did no such thing in cosmos.Entity and per encoding/json default returned a float64.

This meant when we changed client our type assertion failed (silently which is our fault) and broke us.

Should this difference in behavior be documented. (too late to maintain compat?)

@github-actions github-actions bot added Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Service Attention Workflow: This issue is responsible by Azure service team. Tables labels Feb 12, 2025
Copy link

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @klaaslanghout.

@jhendrixMSFT jhendrixMSFT removed the Service Attention Workflow: This issue is responsible by Azure service team. label Feb 12, 2025
@paulgmiller
Copy link
Member Author

paulgmiller commented Feb 12, 2025

Here's a way to reproduce this in a unittest just swap the type of entity to se the type of the property move from float64 to int32.

var raw = map[string]interface{}{
				//"odata.etag": "W/\"datetime'2021-07-10T13%3A45%3A30.1234567Z'\"",
				"PartitionKey":               "partitionA",
				"RowKey":                     "row1",
				"Timestamp":                  "2021-07-10T13:45:30.1234567Z",
				PrimaryTokenIDPropertyName:   "primarycontextid1",
				SecondaryTokenIDPropertyName: "primarycontextid1",
				VNETResourceGUIDPropertyName: "vnetguid1",
				AllocationBlockPrefixSize:    32, //no type info given
			}
			buf, err := json.Marshal(raw)
			Expect(err).NotTo(HaveOccurred())
			//entity := aztables.EDMEntity{}
			entity := cosmos.Entity{}
			Expect(json.Unmarshal(buf, &entity)).To(Succeed())
			switch v := entity.Properties[AllocationBlockPrefixSize].(type) {
			case float64:
				Fail("float64")
			case int32:
				Fail("int32")
			case string:
				Fail("string")
			default:
				Fail("unknown" + fmt.Sprintf("%T", v))
			}

@jhendrixMSFT
Copy link
Member

I'm looking at the docs for Tables and I believe that the legacy storage SDK was in error here.

For numeric entities without an odata.type annotation, int32 is used for numbers without a decimal and float64 (what the default JSON unmarshaler uses) is used when they do.

We can improve the docs on EDMEntity to clarify that numeric values without a decimal will be int32 (it's somewhat documented here but could be clarified).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Client This issue points to a problem in the data-plane of the library. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team Tables
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants