Fix YDB connector semantics, native JOINs and write lifecycle - #270
Merged
Merged
Conversation
Use explicit non-transactional MERGE batches with nullable composite keys. Fix default INSERT staging and reject physical-key updates. Add native JOIN and production-write coverage. Unit validation passes; local integration setup fails because Docker is unavailable.
Match NaN ordering to Trino sort operators and admit Date32 predicates only when their bound values are representable by YDB. Disable the JDBC 2.4.1 shared-context cache by default to avoid a race between connection registration and last-close shutdown. Add a real connector bootstrap check and correct H2 reference-query syntax. All 20 local regressions pass; full real-YDB CI remains required.
Keep signed-minimum modulo by minus one in Trino. Bound Timestamp64 predicate parameters and writes to the native YDB range. Add behavioral filter, projection and JOIN cases plus native timestamp endpoint and just-outside comparisons. All 24 local regressions pass; complete real-YDB CI must validate the new boundary scenarios.
Use an Int64 parameter with CAST(? AS Timestamp64), preserving the full native range and typed NULL values. Add live nullable and NOT NULL endpoint writes. All 24 local regression tests pass.
Avoid the Optional result introduced by SQL CAST. Pass native SDK values through the driver and encode the inclusive maximum without the SDK off-by-one check. Test real required and optional driver conversion. All 24 standalone regression tests pass.
Disable the JDBC IN-to-list rewrite, which bypasses native SDK values. Keep predicate pushdown with scalar parameters and cover both native Timestamp64 endpoints in the live IN regression. All 24 standalone regression tests pass.
KirillKurdyukov
marked this pull request as ready for review
September 30, 2026 17:51
This was referenced Oct 1, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Supersedes the closed connector work in #269 and #267. This PR targets
ydb-platform/ydb-java-dialects, not trinodb/trino.
range preservation, NaN/signed-zero normalization and INNER/LEFT/RIGHT/FULL
coverage. Keep unsafe expressions, conversions and native decimal joins in Trino.
unsigned/decimal writes, UTC temporal mapping and floating Top-N, including
all NULL-placement/direction combinations and representable Date32 predicates.
-1in Trino and bound Timestamp64 predicateparameters and writes. Add the missing behavioral boundary cases found by
independent review. Preserve the inclusive Timestamp64 endpoint through
native SDK values, including NOT NULL writes; disable JDBC 2.4.1's incompatible
IN-list rewrite while retaining IN predicate pushdown.
Make row-level writes explicitly non-transactional and preserve native types,
operation order, nullable composite keys and owned batch resource cleanup.
regressions. Fail rather than silently skip when Docker is unavailable.
per-connection contexts, without retrying writes.
failed CI/review, and the remaining upstream port requirements in
ydb-trino-adapter/UPSTREAM-READINESS.md.Write contract
MERGE and row-level UPDATE/DELETE require
merge.non-transactional-merge.enabled=trueor the corresponding sessionproperty. Previously committed batches can remain after failure or cancellation.
The connector does not claim statement-atomic MERGE or replay failed writes.
Fully pushed UPDATE/DELETE continue to use one YQL RETURNING statement.
Validation
Trino 483, Temurin 25.0.2:
24 passed; 0 failures, errors or skips. This includes actual production
connector bootstrap. All integration sources compiled.
The complete real-YDB CI run
passed for current head
2a443c1909b244e2d102d01414567f6ef9f17ed3:364 tests, 279 passed, 85 skipped, zero failures/errors.
GitHub executes the PR-merge checkout for this head.
This includes the modulo and Timestamp64 boundary gaps found by independent
review. Intermediate runs exposed the SDK endpoint check, SQL CAST nullability,
and optimized IN-list binding; those failures drove the native-value correction
and explicit endpoint IN coverage.
The local full
clean testhad 18 passed and 4 integration-class setupfailures, with 0 errors/skips: the Colima Docker socket was unavailable.
Docker/Colima was not restarted or repaired. The primary checkout was untouched.
Java LSP was unavailable; Maven compilation completed successfully.
The local current-Trino prototype at
dce9fe0f11also passed freshclean verify -P errorprone-compilerwith all 21 unit/API tests. Its full Trino 484integration suite and upstream CI remain unverified; the standalone 483 CI
does not replace them. Legacy JOIN API deprecation warnings and an existing
text-block advisory remain unsuppressed.
Independent review of exact commits
2a443c1909b244e2d102d01414567f6ef9f17ed3and local target
dce9fe0f112e2341d645ed8282dbc61173e78864returned APPROVE,with no remaining blockers. It rechecked both semantic findings from the
earlier whole-connector review, the native SDK/IN parameter paths, current
CI logs and packaged-source identity. Approval covers canonical code-readiness
and the limited local handoff, not unverified Trino 484 runtime or upstream
merge readiness.
Historical upstream CI is not presented as a pass for this branch.
No merge is requested automatically. The actual upstream Trino submission is
left to the designated student's handoff.