Skip to content

YDB-3472: Support StrictSerializableRW and commit timestamps in PHP Query Service - #299

Closed
vgvoleg wants to merge 5 commits into
mainfrom
ydb-3472-strict-serializable-rw
Closed

vgvoleg wants to merge 5 commits into
mainfrom
ydb-3472-strict-serializable-rw

Conversation

@vgvoleg

@vgvoleg vgvoleg commented Sep 30, 2026

Copy link
Copy Markdown
Member

Adds Query Service access through Ydb::queryService() with StrictSerializableRW transaction settings and explicit session and transaction IDs. Exposes optional commit timestamps from explicit commits and the final ExecuteQuery response part as CommitTimestamp values, with access to the original Ydb.VirtualTimestamp protobuf.

Timestamp comparison uses unsigned plan_step and tx_id and accepts values only from the same Ydb connection object. The protobuf has no database identity, so timestamps from separate connections remain incomparable even when their configured paths match. The pinned ydb-api-protos revision already contains the required fields and matches current upstream.

@codecov

codecov Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 55.36%. Comparing base (95214df) to head (e92683e).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #299      +/-   ##
============================================
+ Coverage     52.67%   55.36%   +2.69%     
- Complexity      989     1054      +65     
============================================
  Files            66       69       +3     
  Lines          2690     2841     +151     
============================================
+ Hits           1417     1573     +156     
+ Misses         1273     1268       -5     
Components Coverage Δ
Native SDK 55.36% <100.00%> (+2.69%) ⬆️
Files with missing lines Coverage Δ
src/CommitTimestamp.php 100.00% <100.00%> (ø)
src/QueryExecutionResult.php 100.00% <100.00%> (ø)
src/QueryService.php 100.00% <100.00%> (ø)
src/Ydb.php 76.27% <100.00%> (+0.83%) ⬆️

... and 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@vgvoleg
vgvoleg requested a balanced review from Copilot September 30, 2026 15:08

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@vgvoleg
vgvoleg requested a balanced review from Copilot September 30, 2026 15:19

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟡 Changes recommended

Error recovery can mask failures or invalidate sessions, and timestamp parsing uses an undeclared extension.

Review effort: Balanced
Findings: 1 High severity · 2 Medium severity

Open (3)

Comment thread src/CommitTimestamp.php Outdated
Comment thread src/QueryService.php Outdated
Comment thread src/QueryService.php Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copilot review overview

🟢 Approval recommended

The implementation matches the described API and includes comprehensive coverage of its critical behavior.

Review effort: Balanced
Findings: None

Resolved since last review (3)

@vgvoleg vgvoleg closed this Oct 1, 2026
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.

2 participants