Fix Tables REST serialization, write semantics, and pagination by vaboca · Pull Request #7529 · Azure/azure-sdk-for-cpp

Daniel Boca (vaboca) · 2026-11-06T00:23:23Z

Pull Request Checklist

Please leverage this checklist as a reminder to address commonly occurring feedback when submitting a pull request to make sure your PR can be reviewed quickly:

See the detailed list in the contributing guide.

Description

Correct four related Tables REST behavior gaps:

Write requests: use CRLF, complete embedded HTTP headers, separate each operation from the next MIME boundary, and close both multipart boundaries. InsertReplace sends PUT with JSON and no If-Match; UpdateMerge sends the supplied ETag or *. Point UpdateEntity and MergeEntity also send If-Match: * when the ETag is empty, keeping update behavior distinct from upsert.
Property type identity: infer Edm.Boolean, Edm.Int32, and Edm.Double from native JSON values when annotations are absent. Explicit annotations retain precedence, and unannotated strings remain untyped. This corrects type identity, not numeric precision.
Pagination: retain the original filter, selected columns, and key selector on subsequent pages, and accept partition-only continuation when the next row key is absent or empty.
Entity keys: double apostrophes inside OData key literals before URL encoding in point, query, and batch requests, preserving empty keys and UTF-8 data.

Related to #7184. The regression tests include two deletes with the same partition key and distinct row keys, as well as a delete between JSON-bearing operations. These are offline request-level checks, not a claim of live-service reproduction.

No public API signatures, class layouts, or C++ standard settings change. The existing private merge helper signature is retained. Doxygen documentation and the unreleased Tables changelog are updated.

The changes follow the public contracts for entity group transactions, update preconditions, JSON property types, query continuation, and OData quoting.

Validation

Windows x64, MSVC 19.51, C++14, Debug, WinHTTP enabled, warnings treated as errors. A local-only focused CMake driver selects the repository's unchanged package and test targets; Core and Tables are built from the changed checkout rather than using installed SDK libraries. Tests use synthetic requests and responses and require no Azure credentials or test proxy.

Command	Result
`cmake --build build\tables --target azure-data-tables-test --parallel 4`	Passed
`ctest --test-dir build\tables -R "^azure-data-tables\.(SerializersTest\|TransactionsBodyTest\|TablesRequestTest\|ETags/\|ActionsAndETags/)" --output-on-failure --timeout 30 --output-junit tables-tests.xml`	Passed: 79/79 offline unit tests
`git clang-format --binary clang-format --diff --staged HEAD` (clang-format 20.1.8)	Passed: no changed-line formatting differences
`git diff --cached --check`	Passed

Before the production fixes, these 79 cases produced 45 passes and 34 failures against the unchanged SDK sources. After the fixes, all 79 pass. The failures represent the four behavior families above, not 34 independent defects.

Coverage includes all six transaction actions with empty, wildcard, and conditional ETags; point update/upsert conditions; multipart framing and UTF-8 byte lengths; request order and unchanged raw/error responses; three-page queries with an empty intermediate page; partition-only continuation; key escaping; and native versus explicitly annotated property types.

Service-backed tests, live Azure/authentication, Linux, macOS, ARM64, and libcurl configurations were not run.

Correct multipart framing and update/upsert conditions, preserve inferred property types and query options, and escape OData entity keys. Add native offline regression coverage. Relates to Azure#7184. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

github-actions · 2026-11-06T00:23:42Z

Thank you for your contribution Daniel Boca (@vaboca)! We will review the pull request and get back to you soon.

azure-pipelines · 2026-11-06T00:24:05Z

Azure Pipelines: Successfully started running 3 pipeline(s). 7 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run.

Copilot

🟢 Approval recommended

No active review issues were identified for the Azure Tables C++ REST behavior fixes.

No open issues 🎉

What changed in this PR

Corrects four related REST behaviors in the Azure Tables C++ client, including multipart framing relevant to the multiple-delete failure reported in #7184, without changing public API signatures or class layouts.

Changes:

Fixes transaction framing, write methods and bodies, and update-versus-upsert ETag handling.
Preserves native JSON property types and query options across pages, supports partition-only continuation, and escapes apostrophes in entity keys.
Adds offline regression coverage and updates API documentation and the changelog.

File	Description
sdk/tables/azure-data-tables/test/ut/transactions_test.hpp	Removes service-test dependencies from the transaction fixture.
sdk/tables/azure-data-tables/test/ut/transactions_test.cpp	Updates multipart framing and request-method expectations.
sdk/tables/azure-data-tables/test/ut/table_client_unit_test.cpp	Adds offline write, batch, pagination, and key-escaping tests.
sdk/tables/azure-data-tables/test/ut/serializers_test.hpp	Makes serializer tests independent of service-test setup.
sdk/tables/azure-data-tables/test/ut/serializers_test.cpp	Tests native type inference and explicit annotation precedence.
sdk/tables/azure-data-tables/test/ut/CMakeLists.txt	Registers the new request-level tests.
sdk/tables/azure-data-tables/src/table_clients.cpp	Corrects writes, multipart payloads, pagination, and key escaping.
sdk/tables/azure-data-tables/src/serializers.cpp	Infers property types from unannotated native JSON values.
sdk/tables/azure-data-tables/inc/azure/data/tables/table_client.hpp	Documents empty-ETag behavior and adds a private merge overload.
sdk/tables/azure-data-tables/inc/azure/data/tables/models.hpp	Documents property-type inference rules.
sdk/tables/azure-data-tables/CHANGELOG.md	Records the REST behavior fixes.

_{Review effort: Balanced}

Larry Osterman (LarryOsterman)

Only one efficiency comment.

A description of what an ODATA string type looks like wioth a reference to the spec might help.

Larry Osterman (LarryOsterman) · 2026-11-06T00:50:45Z

+std::string EscapeODataString(std::string const& value)
+{
+  std::string escaped;
+  escaped.reserve(value.size());


This seems incorrect, unless you assume that there are no ' characters in the source. But maybe that is safe.

reserve() is a capacity hint, so the existing loop already doubled apostrophes correctly, but it could allocate again. I changed the reservation to the input length plus the apostrophe count in eb0e740, and added a short explanation of OData string literals with the Table service specification link. The existing point, batch, and paged-query escaping tests pass.

Reserve space for doubled OData apostrophes and document the quoting rule. Keep synthetic test fixtures compatible with the repository spelling check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

maxmarkin01

Thanks, this fixes real problems (multipart framing for #7184, InsertReplace/UpdateMerge semantics, key escaping, and keeping options across pages), and the offline coverage is thorough.

Two items before merge, plus one optional nit:

Type inference combined with string-valued serialization may break Get → Update round trips (inline on serializers.cpp).
The UpdateEntity/MergeEntity empty-ETag change is a breaking behavior change (inline on CHANGELOG.md).

Also, since the multipart framing was rewritten and the transaction/paging service tests are _LIVEONLY_, could we get a Tables live-test pipeline run before merging?

maxmarkin01 · 2026-11-07T03:42:03Z

+      else if (json.at(name).is_boolean())
+      {
+        tableEntity.Properties[name] = TableEntityProperty(value, TableEntityDataType::EdmBoolean);
+      }
+      else if (json.at(name).is_number_integer())
+      {
+        tableEntity.Properties[name] = TableEntityProperty(value, TableEntityDataType::EdmInt32);
+      }
+      else if (json.at(name).is_number_float())
+      {
+        tableEntity.Properties[name] = TableEntityProperty(value, TableEntityDataType::EdmDouble);
+      }


Inferring Edm.Boolean/Edm.Int32/Edm.Double on read changes what a Get → Update round trip sends. CreateEntity always writes Value as a JSON string and adds @odata.type whenever Type is set, so an entity read back from the service is now written as:

"Enabled": "true", "Enabled@odata.type": "Edm.Boolean"

The payload-format docs require Boolean to be a JSON literal and Int32/Double to be JSON numbers; only NaN/Infinity are sent as annotated strings. Before this change the same round trip sent an unannotated string, which silently stored the property as Edm.String (wrong, but it succeeded). After it, the request may be rejected with a 400, and nothing in CI exercises this path. Explicitly typed properties already go through it, but now every round-tripped bool/int/double does.

DeserializeEntityInfersNativePrimitiveTypes only verifies that the SDK can re-read its own output, so it would not catch this.

Could CreateEntity emit native JSON values for these three types, with a unit test asserting the serialized body? Alternatively, a live round-trip test showing the service accepts annotated strings would settle it.

maxmarkin01 · 2026-11-07T03:42:03Z

 ### Bugs Fixed

+- Fixed multipart transaction framing, the `InsertReplace` request method and body, and `UpdateMerge` ETag conditions.
+- Fixed `UpdateEntity` and `MergeEntity` to send `If-Match: *` when the entity's ETag is empty, rather than performing an upsert.


Behavior change worth calling out: previously an empty ETag sent no If-Match, so the service performed Insert-or-Replace/Merge. With If-Match: *, a missing entity now returns 404. This matches the REST contract and the other SDKs, but callers relying on the old behavior will break. Suggest moving the changelog entry under "Breaking Changes" (fine for a beta) and checking the "Any breaking changes?" box.

maxmarkin01 · 2026-11-07T03:42:03Z

+      else if (json.at(name).is_number_integer())
+      {
+        tableEntity.Properties[name] = TableEntityProperty(value, TableEntityDataType::EdmInt32);


Nit (optional): is_number_integer() also matches values outside the Int32 range. The service always sends Int64 as an annotated string, so this shouldn't happen in practice, but leaving out-of-range integers untyped would be a cheap safeguard.

Daniel Boca (vaboca) requested review from Larry Osterman (LarryOsterman) and Anton Kolesnyk (antkmsft) as code owners November 6, 2026 00:23

Copilot AI balanced review requested due to automatic review settings November 6, 2026 00:23

github-actions Bot added Community Contribution Community members are working on the issue customer-reported Issues that are reported by GitHub users external to the Azure organization. Tables labels Nov 6, 2026

Copilot started reviewing on behalf of Daniel Boca (vaboca) November 6, 2026 00:23

View session

Copilot AI reviewed Nov 6, 2026

View reviewed changes

Anton Kolesnyk (antkmsft) approved these changes Nov 6, 2026

View reviewed changes

Larry Osterman (LarryOsterman) approved these changes Nov 6, 2026

View reviewed changes

fix(tables): address quoting review and spelling checks

eb0e740

Reserve space for doubled OData apostrophes and document the quoting rule. Keep synthetic test fixtures compatible with the repository spelling check. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

maxmarkin01 approved these changes Nov 7, 2026

View reviewed changes

maxmarkin01 reviewed Nov 7, 2026

View reviewed changes

Anton Kolesnyk (antkmsft) approved these changes Nov 7, 2026

View reviewed changes

Navigation Menu

Repository navigation

Fix Tables REST serialization, write semantics, and pagination - #7529

Fix Tables REST serialization, write semantics, and pagination#7529
Daniel Boca (vaboca) wants to merge 2 commits into
Azure:mainfrom
vaboca:personal/vaboca/x-upstream-tables-fixes

Daniel Boca (vaboca) commented Nov 6, 2026

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

azure-pipelines Bot commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Larry Osterman (LarryOsterman) left a comment

Uh oh!

Larry Osterman (LarryOsterman) Nov 6, 2026

Uh oh!

Daniel Boca (vaboca) Nov 6, 2026

Uh oh!

maxmarkin01 left a comment

Uh oh!

maxmarkin01 Nov 7, 2026

Uh oh!

maxmarkin01 Nov 7, 2026

Uh oh!

maxmarkin01 Nov 7, 2026

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

Daniel Boca (vaboca) commented Nov 6, 2026

Pull Request Checklist

Description

Validation

Uh oh!

github-actions Bot commented Nov 6, 2026

Uh oh!

azure-pipelines Bot commented Nov 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

🟢 Approval recommended

Uh oh!

Larry Osterman (LarryOsterman) left a comment

Choose a reason for hiding this comment

Uh oh!

Larry Osterman (LarryOsterman) Nov 6, 2026

Choose a reason for hiding this comment

Uh oh!

Daniel Boca (vaboca) Nov 6, 2026

Choose a reason for hiding this comment

Uh oh!

maxmarkin01 left a comment

Choose a reason for hiding this comment

Uh oh!

maxmarkin01 Nov 7, 2026

Choose a reason for hiding this comment

Uh oh!

maxmarkin01 Nov 7, 2026

Choose a reason for hiding this comment

Uh oh!

maxmarkin01 Nov 7, 2026

Choose a reason for hiding this comment

Uh oh!

This branch has not been deployed

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants