From 905272e0200dc584f1a160dff8b0284805374946 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Tue, 29 Sep 2026 17:01:34 +0300 Subject: [PATCH 01/13] Add opt-in Int64 equality JOIN pushdown for YDB --- ydb-trino-adapter/README.md | 8 ++++ .../main/java/tech/ydb/trino/YdbClient.java | 32 +++++++++++++ .../main/java/tech/ydb/trino/YdbPlugin.java | 4 ++ .../tech/ydb/trino/TestYdbConnectorTest.java | 46 +++++++++++++++++++ 4 files changed, 90 insertions(+) diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index e4aad3c5..c3a5ff36 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -64,6 +64,14 @@ AS SELECT tenant, event_id, payload FROM events; integration-проверка CTAS использует `insert.non-transactional-insert.enabled=true`; transactional staging этой проверкой не подтверждается. +## JOIN pushdown + +По умолчанию JOIN выполняет Trino. Для пробного pushdown задайте +`join_pushdown_enabled=true` в сессии каталога. Адаптер передаёт YDB +`INNER`, `LEFT`, `RIGHT` и `FULL JOIN` только по равенству исходных столбцов +`Int64` (Trino `bigint`), включая составной ключ. Остальные условия JOIN, +вычисляемые ключи и другие типы остаются в Trino. См. [правила YQL JOIN](https://ydb.tech/docs/ru/yql/reference/syntax/select/join). + ## Текст и байты YDB `Text` отображается в Trino как `varchar`, а `Bytes` — как `varbinary` без diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index 87d7f8ae..ed2f6bfc 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -18,6 +18,7 @@ import io.trino.plugin.jdbc.ConnectionFactory; import io.trino.plugin.jdbc.JdbcColumnHandle; import io.trino.plugin.jdbc.JdbcExpression; +import io.trino.plugin.jdbc.JdbcJoinCondition; import io.trino.plugin.jdbc.JdbcMergeTableHandle; import io.trino.plugin.jdbc.JdbcOutputTableHandle; import io.trino.plugin.jdbc.JdbcSortItem; @@ -44,6 +45,8 @@ import io.trino.spi.connector.ColumnMetadata; import io.trino.spi.connector.ConnectorSession; import io.trino.spi.connector.ConnectorTableMetadata; +import io.trino.spi.connector.JoinStatistics; +import io.trino.spi.connector.JoinType; import io.trino.spi.connector.RetryMode; import io.trino.spi.connector.SchemaTableName; import io.trino.spi.connector.SortOrder; @@ -102,6 +105,7 @@ import static io.trino.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; +import static io.trino.spi.connector.JoinCondition.Operator.EQUAL; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.BooleanType.BOOLEAN; import static io.trino.spi.type.DateType.DATE; @@ -199,6 +203,34 @@ public YdbClient( .build()); } + @Override + protected boolean isSupportedJoinCondition(ConnectorSession session, JdbcJoinCondition condition) { + return condition.getOperator() == EQUAL + && isNativeInt64JoinKey(condition.getLeftColumn()) + && isNativeInt64JoinKey(condition.getRightColumn()); + } + + private boolean isNativeInt64JoinKey(JdbcColumnHandle column) { + return column.getColumnType().equals(BIGINT) + && column.getJdbcTypeHandle().jdbcTypeName().filter("Int64"::equalsIgnoreCase).isPresent() + && getForcedMappingToVarchar(column.getJdbcTypeHandle()).isEmpty() + && !column.getComment().filter("synthetic"::equals).isPresent(); + } + + @Override + public Optional implementJoin( + ConnectorSession session, + JoinType joinType, + PreparedQuery leftSource, + Map leftProjections, + PreparedQuery rightSource, + Map rightProjections, + List joinConditions, + JoinStatistics statistics) { + // The expression API loses which source owns each YQL ON operand. + return Optional.empty(); + } + @Override public Optional implementAggregation( ConnectorSession session, diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPlugin.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPlugin.java index 5ef33be0..b44488e9 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPlugin.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPlugin.java @@ -2,11 +2,13 @@ import com.google.common.collect.ImmutableList; import com.google.inject.Module; +import io.trino.plugin.jdbc.JdbcMetadataConfig; import io.trino.plugin.jdbc.credential.CredentialProviderModule; import io.trino.spi.Plugin; import io.trino.spi.connector.ConnectorFactory; import static io.airlift.configuration.ConfigurationAwareModule.combine; +import static io.airlift.configuration.ConfigBinder.configBinder; public record YdbPlugin(Module module) implements Plugin { private static final String NAME = "ydb"; @@ -21,6 +23,8 @@ public Iterable getConnectorFactories() { NAME, () -> combine( new CredentialProviderModule(), + binder -> configBinder(binder).bindConfigDefaults( + JdbcMetadataConfig.class, config -> config.setComplexJoinPushdownEnabled(false)), module ) )); diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java index fdde9656..6537101d 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java @@ -1,5 +1,6 @@ package tech.ydb.trino; +import io.trino.Session; import io.trino.spi.type.Type; import io.trino.spi.type.VarcharType; import io.trino.testing.BaseConnectorTest; @@ -13,6 +14,7 @@ import org.junit.jupiter.api.extension.RegisterExtension; import tech.ydb.test.junit5.YdbHelperExtension; +import java.util.List; import java.util.Optional; import java.util.OptionalInt; @@ -42,6 +44,50 @@ public void testDefaultSchema() { assertQueryFails("SELECT * FROM local.\"%\".orders", ".*Schema '%' does not exist"); } + @Test + public void testOptInInt64JoinPushdown() { + Session session = Session.builder(getSession()) + .setCatalogSessionProperty(getSession().getCatalog().orElseThrow(), "join_pushdown_enabled", "true") + .build(); + try (TestTable left = newTrinoTable("join_int64_left_", "(id bigint, k bigint, second_key bigint, d double, s varchar)", + List.of("1, 7, 1, 1.0, 'a'", "2, 7, 2, 2.0, 'b'", "3, NULL, 1, 3.0, 'c'", "4, 8, 1, 4.0, 'd'")); + TestTable right = newTrinoTable("join_int64_right_", "(id bigint, k bigint, second_key bigint, d double, s varchar)", + List.of("10, 7, 1, 1.0, 'a'", "11, 7, 2, 2.0, 'b'", "12, NULL, 1, 3.0, 'c'", "13, 9, 1, 4.0, 'd'"))) { + String join = "SELECT l.id, r.id FROM " + left.getName() + " l %s " + right.getName() + " r ON %s"; + String matches = "VALUES (BIGINT '1', BIGINT '10'), (BIGINT '1', BIGINT '11'), " + + "(BIGINT '2', BIGINT '10'), (BIGINT '2', BIGINT '11')"; + assertThat(query(session, join.formatted("JOIN", "l.k = r.k"))).isFullyPushedDown().matches(matches); + assertThat(query(session, join.formatted("LEFT JOIN", "l.k = r.k"))).isFullyPushedDown() + .matches(matches + ", (BIGINT '3', CAST(NULL AS BIGINT)), (BIGINT '4', CAST(NULL AS BIGINT))"); + assertThat(query(session, join.formatted("RIGHT JOIN", "l.k = r.k"))).isFullyPushedDown() + .matches(matches + ", (CAST(NULL AS BIGINT), BIGINT '12'), (CAST(NULL AS BIGINT), BIGINT '13')"); + assertThat(query(session, join.formatted("FULL JOIN", "l.k = r.k"))).isFullyPushedDown() + .matches(matches + ", (BIGINT '3', CAST(NULL AS BIGINT)), (BIGINT '4', CAST(NULL AS BIGINT)), " + + "(CAST(NULL AS BIGINT), BIGINT '12'), (CAST(NULL AS BIGINT), BIGINT '13')"); + + assertThat(query(session, join.formatted("JOIN", "l.k = r.k AND l.second_key = r.second_key"))) + .isFullyPushedDown() + .matches("VALUES (BIGINT '1', BIGINT '10'), (BIGINT '2', BIGINT '11')"); + assertThat(query(session, "SELECT l.id, r.id FROM (SELECT id, k FROM " + left.getName() + + " WHERE id > 1 AND id < 3) l JOIN (SELECT id, k FROM " + right.getName() + + " WHERE id > 10 AND id < 12) r ON l.k = r.k")) + .isFullyPushedDown() + .matches("VALUES (BIGINT '2', BIGINT '11')"); + + assertThat(query(getSession(), join.formatted("JOIN", "l.k = r.k"))).joinIsNotFullyPushedDown(); + assertThat(query(session, join.formatted("JOIN", "l.k < r.k"))) + .joinIsNotFullyPushedDown(); + assertThat(query(session, join.formatted("JOIN", "l.s = r.s"))) + .joinIsNotFullyPushedDown(); + assertThat(query(session, join.formatted("JOIN", "l.d = r.d"))) + .joinIsNotFullyPushedDown(); + Session complex = Session.builder(session) + .setCatalogSessionProperty(getSession().getCatalog().orElseThrow(), "complex_join_pushdown_enabled", "true") + .build(); + assertThat(query(complex, join.formatted("JOIN", "l.k = r.k"))).joinIsNotFullyPushedDown(); + } + } + @Override protected void verifyConcurrentAddColumnFailurePermissible(Exception exception) { // YDB serializes overlapping scheme operations on one table and reports the conflict as retryable OVERLOADED: From 8560824cc94ae00127a8f22daef0f27aeb0be757 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Tue, 29 Sep 2026 17:31:25 +0300 Subject: [PATCH 02/13] Add YDB JOIN key mapping and qualified YQL builder --- .../main/java/tech/ydb/trino/YdbClient.java | 26 ++++++++---- .../java/tech/ydb/trino/YdbClientModule.java | 5 +++ .../java/tech/ydb/trino/YdbQueryBuilder.java | 41 +++++++++++++++++++ .../tech/ydb/trino/TestingYdbJdbcModule.java | 5 +++ 4 files changed, 68 insertions(+), 9 deletions(-) create mode 100644 ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index ed2f6bfc..29407615 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -206,15 +206,23 @@ public YdbClient( @Override protected boolean isSupportedJoinCondition(ConnectorSession session, JdbcJoinCondition condition) { return condition.getOperator() == EQUAL - && isNativeInt64JoinKey(condition.getLeftColumn()) - && isNativeInt64JoinKey(condition.getRightColumn()); - } - - private boolean isNativeInt64JoinKey(JdbcColumnHandle column) { - return column.getColumnType().equals(BIGINT) - && column.getJdbcTypeHandle().jdbcTypeName().filter("Int64"::equalsIgnoreCase).isPresent() - && getForcedMappingToVarchar(column.getJdbcTypeHandle()).isEmpty() - && !column.getComment().filter("synthetic"::equals).isPresent(); + && hasSupportedValueMapping(condition.getLeftColumn()) + && hasSupportedValueMapping(condition.getRightColumn()); + } + + private boolean hasSupportedValueMapping(JdbcColumnHandle column) { + JdbcTypeHandle type = column.getJdbcTypeHandle(); + if (getForcedMappingToVarchar(type).isPresent()) { + return false; + } + String name = type.jdbcTypeName().orElse("").toLowerCase(Locale.ROOT); + return switch (name) { + case "bool", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64", + "float", "double", "utf8", "text", "date", "date32", "datetime", "datetime64", + "timestamp", "timestamp64", "decimal" -> true; + case "string", "bytes" -> column.getColumnType().equals(VARBINARY); + default -> name.startsWith("decimal("); + }; } @Override diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java index 46d8f723..bb6e18fe 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java @@ -24,6 +24,11 @@ public class YdbClientModule implements Module { @Override public void configure(Binder binder) { + newOptionalBinder(binder, QueryBuilder.class) + .setBinding() + .to(YdbQueryBuilder.class) + .in(Scopes.SINGLETON); + newOptionalBinder(binder, JdbcMetadataFactory.class) .setBinding() .to(YdbMetadataFactory.class) diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java new file mode 100644 index 00000000..6ab88d69 --- /dev/null +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java @@ -0,0 +1,41 @@ +package tech.ydb.trino; + +import com.google.inject.Inject; +import io.trino.plugin.jdbc.DefaultQueryBuilder; +import io.trino.plugin.jdbc.JdbcClient; +import io.trino.plugin.jdbc.JdbcColumnHandle; +import io.trino.plugin.jdbc.JdbcJoinCondition; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; + +import static io.trino.spi.type.DoubleType.DOUBLE; +import static io.trino.spi.type.RealType.REAL; +import static java.lang.String.format; + +public class YdbQueryBuilder extends DefaultQueryBuilder { + @Inject + public YdbQueryBuilder(RemoteQueryModifier queryModifier) { + super(queryModifier); + } + + @Override + protected String formatJoinCondition(JdbcClient client, String leftAlias, String rightAlias, JdbcJoinCondition condition) { + return format("%s %s %s", + formatJoinKey(client, leftAlias, condition.getLeftColumn()), + condition.getOperator().getValue(), + formatJoinKey(client, rightAlias, condition.getRightColumn())); + } + + private String formatJoinKey(JdbcClient client, String alias, JdbcColumnHandle column) { + String reference = alias + "." + client.quoted(column.getColumnName()); + if (column.getJdbcTypeHandle().jdbcTypeName().filter("Uint64"::equalsIgnoreCase).isPresent()) { + return "BITCAST(" + reference + " AS Int64)"; + } + if (column.getColumnType().equals(REAL) || column.getColumnType().equals(DOUBLE)) { + // Match Trino's JOIN behavior for NaN and signed zero. + String type = column.getColumnType().equals(REAL) ? "Float" : "Double"; + return format("NANVL(IF(COALESCE(%1$s = 0, false), CAST(0 AS %2$s), %1$s), CAST(NULL AS %2$s))", + reference, type); + } + return reference; + } +} diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java index 9b380331..3d947848 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java @@ -22,6 +22,11 @@ public class TestingYdbJdbcModule implements Module { @Override public void configure(Binder binder) { + newOptionalBinder(binder, QueryBuilder.class) + .setBinding() + .to(YdbQueryBuilder.class) + .in(Scopes.SINGLETON); + newOptionalBinder(binder, JdbcMetadataFactory.class) .setBinding() .to(YdbMetadataFactory.class) From dbc67b056ef0a15d2aa3f9d0aa1658428d272a79 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Tue, 29 Sep 2026 17:42:24 +0300 Subject: [PATCH 03/13] Assert newly admitted scalar JOIN keys --- .../src/test/java/tech/ydb/trino/TestYdbConnectorTest.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java index 6537101d..f8f7779f 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java @@ -78,9 +78,12 @@ public void testOptInInt64JoinPushdown() { assertThat(query(session, join.formatted("JOIN", "l.k < r.k"))) .joinIsNotFullyPushedDown(); assertThat(query(session, join.formatted("JOIN", "l.s = r.s"))) - .joinIsNotFullyPushedDown(); + .matches("VALUES (BIGINT '1', BIGINT '10'), (BIGINT '2', BIGINT '11')") + .isFullyPushedDown(); assertThat(query(session, join.formatted("JOIN", "l.d = r.d"))) - .joinIsNotFullyPushedDown(); + .matches("VALUES (BIGINT '1', BIGINT '10'), (BIGINT '2', BIGINT '11'), " + + "(BIGINT '3', BIGINT '12'), (BIGINT '4', BIGINT '13')") + .isFullyPushedDown(); Session complex = Session.builder(session) .setCatalogSessionProperty(getSession().getCatalog().orElseThrow(), "complex_join_pushdown_enabled", "true") .build(); From 0bf7606e31845cd1a1048dcf24888d007eef5c2d Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Tue, 29 Sep 2026 18:02:01 +0300 Subject: [PATCH 04/13] Expect all matching text keys in JOIN fixture --- .../src/test/java/tech/ydb/trino/TestYdbConnectorTest.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java index f8f7779f..8e9f2927 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java @@ -78,7 +78,8 @@ public void testOptInInt64JoinPushdown() { assertThat(query(session, join.formatted("JOIN", "l.k < r.k"))) .joinIsNotFullyPushedDown(); assertThat(query(session, join.formatted("JOIN", "l.s = r.s"))) - .matches("VALUES (BIGINT '1', BIGINT '10'), (BIGINT '2', BIGINT '11')") + .matches("VALUES (BIGINT '1', BIGINT '10'), (BIGINT '2', BIGINT '11'), " + + "(BIGINT '3', BIGINT '12'), (BIGINT '4', BIGINT '13')") .isFullyPushedDown(); assertThat(query(session, join.formatted("JOIN", "l.d = r.d"))) .matches("VALUES (BIGINT '1', BIGINT '10'), (BIGINT '2', BIGINT '11'), " + From 0fe5564963ce63edf4537a703cffe8b235e35e7c Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 16:45:58 +0300 Subject: [PATCH 05/13] Fix YDB expression parameter order and overflow fallback --- ydb-trino-adapter/README.md | 6 ++ .../tech/ydb/trino/RewriteDivideModulus.java | 26 +++---- .../java/tech/ydb/trino/RewriteNullIf.java | 21 ++++-- .../tech/ydb/trino/RewriteStringPosition.java | 12 ++-- .../main/java/tech/ydb/trino/YdbClient.java | 4 -- .../ydb/trino/TestYdbExpressionRewrites.java | 72 +++++++++++++++++++ 6 files changed, 113 insertions(+), 28 deletions(-) create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index c3a5ff36..65ceedb5 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -74,6 +74,12 @@ transactional staging этой проверкой не подтверждает ## Текст и байты +Вычисления целочисленного сложения, вычитания, умножения и отрицания остаются +в Trino: переполнение в YQL не обязано приводить к той же ошибке. Деление +передаётся в YDB только для целочисленного результата с ненулевым постоянным +делителем, отличным от `-1`. `strpos` считает позиции Unicode-символов и +сохраняет SQL `NULL`, а не заменяет его нулём. + YDB `Text` отображается в Trino как `varchar`, а `Bytes` — как `varbinary` без декодирования UTF-8. При создании таблиц адаптер использует типы `Text` и `Bytes`. diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java index 015b5a67..6e05ca05 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java @@ -5,19 +5,25 @@ import io.trino.plugin.base.expression.ConnectorExpressionRule; import io.trino.plugin.jdbc.expression.ParameterizedExpression; import io.trino.spi.expression.Call; +import io.trino.spi.expression.Constant; import java.util.Optional; import static io.trino.plugin.base.expression.ConnectorExpressionPatterns.argumentCount; import static io.trino.plugin.base.expression.ConnectorExpressionPatterns.call; import static io.trino.plugin.base.expression.ConnectorExpressionPatterns.functionName; +import static io.trino.plugin.base.expression.ConnectorExpressionPatterns.type; import static io.trino.spi.expression.StandardFunctions.DIVIDE_FUNCTION_NAME; import static io.trino.spi.expression.StandardFunctions.MODULO_FUNCTION_NAME; +import static io.trino.spi.type.BigintType.BIGINT; +import static io.trino.spi.type.IntegerType.INTEGER; +import static io.trino.spi.type.SmallintType.SMALLINT; +import static io.trino.spi.type.TinyintType.TINYINT; import static java.lang.String.format; /** * Rewrite $divide(a, b), $modulus(a, b) as a / b, a % b, - * but only if b is a non-zero constant. + * for integral operands with a non-zero constant divisor, excluding division by -1. *

* This is because YDB, unlike Trino, suppresses arithmetic errors (including division by zero), and any * non-constant expression may evaluate to zero and cause a different result if pushed down to YDB. @@ -29,6 +35,8 @@ public RewriteDivideModulus() { this.PATTERN = call() .with(functionName().matching(name -> name.equals(DIVIDE_FUNCTION_NAME) || name.equals(MODULO_FUNCTION_NAME))) .with(argumentCount().matching(count -> count == 2)) + .with(type().matching(resultType -> resultType.equals(TINYINT) || resultType.equals(SMALLINT) || + resultType.equals(INTEGER) || resultType.equals(BIGINT))) .matching((Call call, RewriteContext _) -> call.getArguments().stream().noneMatch(arg -> arg instanceof Call)); } @@ -40,24 +48,16 @@ public Pattern getPattern() { @Override public Optional rewrite(Call call, Captures captures, RewriteContext context) { - String operator = getOperator(call.getFunctionName()); + String operator = call.getFunctionName().equals(DIVIDE_FUNCTION_NAME) ? "/" : "%"; return RewriteUtils.rewriteBinaryExpression( call, context, - () -> call.getArguments().get(1) instanceof io.trino.spi.expression.Constant rightConstant && + () -> call.getArguments().get(1) instanceof Constant rightConstant && rightConstant.getValue() instanceof Number number && - number.longValue() != 0, + number.longValue() != 0 && + (!call.getFunctionName().equals(DIVIDE_FUNCTION_NAME) || number.longValue() != -1), (left, right) -> format("(%s) %s (%s)", left, operator, right) ); } - private String getOperator(io.trino.spi.expression.FunctionName functionName) { - if (functionName.equals(DIVIDE_FUNCTION_NAME)) { - return "/"; - } - if (functionName.equals(MODULO_FUNCTION_NAME)) { - return "%"; - } - return null; - } } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteNullIf.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteNullIf.java index 30181aea..94924648 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteNullIf.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteNullIf.java @@ -1,8 +1,10 @@ package tech.ydb.trino; +import com.google.common.collect.ImmutableList; import io.trino.matching.Captures; import io.trino.matching.Pattern; import io.trino.plugin.base.expression.ConnectorExpressionRule; +import io.trino.plugin.jdbc.QueryParameter; import io.trino.plugin.jdbc.expression.ParameterizedExpression; import io.trino.spi.expression.Call; import io.trino.spi.expression.StandardFunctions; @@ -35,11 +37,18 @@ public Pattern getPattern() { @Override public Optional rewrite(Call call, Captures captures, RewriteContext context) { - return RewriteUtils.rewriteBinaryExpression( - call, - context, - () -> true, - (left, right) -> format("CASE WHEN %s = %s THEN NULL ELSE %s END", left, right, left) - ); + Optional left = context.defaultRewrite(call.getArguments().getFirst()); + Optional right = context.defaultRewrite(call.getArguments().get(1)); + if (left.isEmpty() || right.isEmpty()) { + return Optional.empty(); + } + return Optional.of(new ParameterizedExpression( + format("CASE WHEN %s = %s THEN NULL ELSE %s END", + left.get().expression(), right.get().expression(), left.get().expression()), + ImmutableList.builder() + .addAll(left.get().parameters()) + .addAll(right.get().parameters()) + .addAll(left.get().parameters()) + .build())); } } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteStringPosition.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteStringPosition.java index b39bf5ed..32ababee 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteStringPosition.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteStringPosition.java @@ -27,9 +27,9 @@ /** * Rewrite strpos(str, sub), as: *
- * CASE WHEN Find(str, sub) IS NULL THEN 0 ELSE Find(str, sub) + 1 END. + * Unicode::Find(str, sub) + 1, with SQL null and missing-substring handling. *

- * The + 1 is due to the fact that Trino uses 0-based indexing while YDB uses 1-based indexing. + * YQL positions are zero-based; Trino positions are one-based Unicode code points. */ public class RewriteStringPosition implements ProjectFunctionRule { private static final Capture STRING = newCapture(); @@ -69,12 +69,14 @@ public Optional rewrite( ImmutableList.Builder<@NonNull QueryParameter> parameters = ImmutableList.builder(); parameters.addAll(rewrittenString.get().parameters()); - // Add substring parameters twice - for both occurrences in CASE expression parameters.addAll(rewrittenSubstring.get().parameters()); + parameters.addAll(rewrittenString.get().parameters()); parameters.addAll(rewrittenSubstring.get().parameters()); - String findExpr = format("Find(%s, %s)", strSql, subSql); - String expression = format("CASE WHEN (%s) IS NULL THEN 0 ELSE (%s) + 1 END", findExpr, findExpr); + String expression = format( + "CASE WHEN (%s) IS NULL OR (%s) IS NULL THEN CAST(NULL AS Int64) " + + "ELSE COALESCE(CAST(Unicode::Find(Unwrap(%s), Unwrap(%s)) AS Int64) + 1, 0) END", + strSql, subSql, strSql, subSql); JdbcExpression result = new JdbcExpression( expression, diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index 29407615..15574a34 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -169,10 +169,6 @@ public YdbClient( "tinyint", "smallint", "integer", "bigint", "decimal", "real", "double", "varchar", "char", "date", "timestamp")) .map("$equal(left, right)").to("left = right") .map("$not_equal(left, right)").to("left <> right") - .map("$add(left: integer_type, right: integer_type)").to("left + right") - .map("$subtract(left: integer_type, right: integer_type)").to("left - right") - .map("$multiply(left: integer_type, right: integer_type)").to("left * right") - .map("$negate(value: integer_type)").to("-value") .map("$less_than(left: comparable_type, right: comparable_type)").to("left < right") .map("$less_than_or_equal(left: comparable_type, right: comparable_type)").to("left <= right") .map("$greater_than(left: comparable_type, right: comparable_type)").to("left > right") diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java new file mode 100644 index 00000000..840ccac5 --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java @@ -0,0 +1,72 @@ +package tech.ydb.trino; + +import io.trino.plugin.base.mapping.DefaultIdentifierMapping; +import io.trino.plugin.jdbc.BaseJdbcConfig; +import io.trino.plugin.jdbc.JdbcColumnHandle; +import io.trino.plugin.jdbc.JdbcTableHandle; +import io.trino.plugin.jdbc.RemoteTableName; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.expression.Call; +import io.trino.spi.expression.Constant; +import io.trino.spi.expression.FunctionName; +import io.trino.spi.expression.Variable; +import org.junit.jupiter.api.Test; + +import java.sql.SQLException; +import java.util.List; +import java.util.Map; +import java.util.Optional; + +import static io.airlift.slice.Slices.utf8Slice; +import static io.trino.spi.expression.StandardFunctions.ADD_FUNCTION_NAME; +import static io.trino.spi.expression.StandardFunctions.DIVIDE_FUNCTION_NAME; +import static io.trino.spi.expression.StandardFunctions.NULLIF_FUNCTION_NAME; +import static io.trino.spi.type.BigintType.BIGINT; +import static io.trino.spi.type.VarcharType.VARCHAR; +import static io.trino.testing.TestingConnectorSession.SESSION; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestYdbExpressionRewrites { + private final YdbClient client = new YdbClient( + new BaseJdbcConfig(), + _ -> { + throw new SQLException("This test must not open a connection"); + }, + new YdbQueryBuilder(RemoteQueryModifier.NONE), + new DefaultIdentifierMapping(), + RemoteQueryModifier.NONE); + + @Test + public void testNullIfParameterOrder() { + Call expression = new Call(BIGINT, NULLIF_FUNCTION_NAME, List.of( + new Constant(11L, BIGINT), new Variable("v", BIGINT))); + JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(BIGINT).orElseThrow(), BIGINT); + var result = client.convertPredicate(SESSION, expression, Map.of("v", column)).orElseThrow(); + assertThat(result.parameters()).extracting(parameter -> parameter.getValue().orElseThrow()) + .containsExactly(11L, 11L); + } + + @Test + public void testStringPositionParameterOrder() { + Call expression = new Call(BIGINT, new FunctionName("strpos"), List.of( + new Constant(utf8Slice("hello"), VARCHAR), new Variable("v", VARCHAR))); + JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(VARCHAR).orElseThrow(), VARCHAR); + JdbcTableHandle table = new JdbcTableHandle( + new SchemaTableName("default", "test"), + new RemoteTableName(Optional.empty(), Optional.empty(), "test"), + Optional.empty()); + var result = client.convertProjection(SESSION, table, expression, Map.of("v", column)).orElseThrow(); + assertThat(result.getParameters()).extracting(parameter -> parameter.getValue().orElseThrow()) + .containsExactly(utf8Slice("hello"), utf8Slice("hello")); + } + + @Test + public void testOverflowingArithmeticIsNotPushedDown() { + JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(BIGINT).orElseThrow(), BIGINT); + assertThat(client.convertPredicate(SESSION, new Call(BIGINT, ADD_FUNCTION_NAME, List.of( + new Variable("v", BIGINT), new Constant(1L, BIGINT))), Map.of("v", column))).isEmpty(); + assertThat(client.convertPredicate(SESSION, new Call(BIGINT, DIVIDE_FUNCTION_NAME, List.of( + new Variable("v", BIGINT), new Constant(-1L, BIGINT))), Map.of("v", column))).isEmpty(); + } +} From e4b4e2c13086c377470cf8fc44f25c04a8bed9be Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 17:04:28 +0300 Subject: [PATCH 06/13] Preserve YDB scalar mappings and equality JOIN semantics --- ydb-trino-adapter/README.md | 28 +++- .../main/java/tech/ydb/trino/YdbClient.java | 133 ++++++++++++++--- .../tech/ydb/trino/YdbColumnMappings.java | 124 ++++++++++++++-- .../java/tech/ydb/trino/YdbQueryBuilder.java | 2 +- .../java/tech/ydb/trino/YdbTypeUtils.java | 45 +++--- .../tech/ydb/trino/TestYdbColumnMappings.java | 135 ++++++++++++++++++ .../tech/ydb/trino/TestYdbJoinMappings.java | 88 ++++++++++++ 7 files changed, 500 insertions(+), 55 deletions(-) create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index 65ceedb5..2cadb69f 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -69,8 +69,27 @@ transactional staging этой проверкой не подтверждает По умолчанию JOIN выполняет Trino. Для пробного pushdown задайте `join_pushdown_enabled=true` в сессии каталога. Адаптер передаёт YDB `INNER`, `LEFT`, `RIGHT` и `FULL JOIN` только по равенству исходных столбцов -`Int64` (Trino `bigint`), включая составной ключ. Остальные условия JOIN, -вычисляемые ключи и другие типы остаются в Trino. См. [правила YQL JOIN](https://ydb.tech/docs/ru/yql/reference/syntax/select/join). +с совместимыми отображениями: `Bool`, знаковые и беззнаковые целые, +`Float`/`Double`, текст, байты, даты и timestamps. Поддерживается составной ключ. +`NULL` и `NaN` не совпадают по обычному равенству; `-0.0` совпадает с `0.0`. +Для YQL ключи с `NaN` заменяются на `NULL`, а нули нормализуются. +Native `Decimal`, null-safe equality, неравенства, вычисляемые ключи и +неподдерживаемые преобразования остаются в Trino. +См. [правила YQL JOIN](https://ydb.tech/docs/ru/yql/reference/syntax/select/join). + +## Числовые типы + +`Uint8`, `Uint16`, `Uint32` отображаются соответственно в `smallint`, +`integer`, `bigint`. `Uint64` отображается в `decimal(20,0)` и сохраняет +весь диапазон до `18446744073709551615`, без превращения старшего бита в знак. +Запись проверяет границы unsigned-типа. `Decimal` записывается с точными +precision и scale, включая `NULL`; максимальная поддерживаемая precision — 35. +Native decimal `NaN` и бесконечности не представимы в Trino `decimal`. + +Целочисленные и decimal `sum`/`avg`, а также группировки и `min`/`max` +для floating-point выполняет Trino там, где YQL отличается по переполнению, +порядку `NaN` или знаковому нулю. Top-N явно учитывает SQL `NULL` и порядок +`NaN`, используя типизированные floating-point значения в YQL. ## Текст и байты @@ -90,8 +109,11 @@ YDB `Text` отображается в Trino как `varchar`, а `Bytes` — к Новые столбцы Trino `timestamp(3)` и `timestamp(6)` создаются как YDB `Timestamp64`. Существующие YDB `Datetime`, `Datetime64`, `Timestamp` и `Timestamp64` читаются как Trino `timestamp(6)`; исходная точность `timestamp(3)` в метаданных не сохраняется. +Чтение timestamps использует UTC, а не часовой пояс JVM. Запись дробных секунд +в `Datetime`/`Datetime64` отклоняется без молчаливого усечения. При записи адаптер передаёт YDB JDBC точный vendor type по `TYPE_NAME` и больше не задаёт `forceSignedDatetimes`: `Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp`/`Timestamp64` — `Instant` с микросекундами. В `UPDATE`/`DELETE`, которые стандартный merge sink выполняет внутри `MERGE`, native `TYPE_NAME` не передаётся; fallback `Instant` для `Datetime`/`Datetime64` может зависеть от часового пояса JVM и остаётся отдельным риском. -Диапазонные предикаты вне диапазона legacy YDB `Date` остаются отдельным известным ограничением. +Предикаты для `Date`/`Date32` и legacy temporal типов оставляются в Trino, +чтобы не связывать значения вне диапазона исходного YDB-типа. diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index 15574a34..26016ad5 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -15,6 +15,8 @@ import io.trino.plugin.jdbc.BaseJdbcConfig; import io.trino.plugin.jdbc.BooleanWriteFunction; import io.trino.plugin.jdbc.ColumnMapping; +import io.trino.plugin.jdbc.LongWriteFunction; +import io.trino.plugin.jdbc.ObjectWriteFunction; import io.trino.plugin.jdbc.ConnectionFactory; import io.trino.plugin.jdbc.JdbcColumnHandle; import io.trino.plugin.jdbc.JdbcExpression; @@ -51,6 +53,8 @@ import io.trino.spi.connector.SchemaTableName; import io.trino.spi.connector.SortOrder; import io.trino.spi.expression.ConnectorExpression; +import io.trino.spi.expression.Constant; +import io.trino.spi.expression.Variable; import io.trino.spi.type.DecimalType; import io.trino.spi.type.Type; import io.trino.spi.type.VarcharType; @@ -86,7 +90,6 @@ import static io.trino.plugin.jdbc.StandardColumnMappings.bigintColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.bigintWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.booleanColumnMapping; -import static io.trino.plugin.jdbc.StandardColumnMappings.decimalColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.doubleColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.doubleWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.integerColumnMapping; @@ -98,11 +101,14 @@ import static io.trino.plugin.jdbc.StandardColumnMappings.smallintColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.smallintWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.tinyintWriteFunction; +import static io.trino.plugin.jdbc.StandardColumnMappings.tinyintColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.varbinaryColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.varbinaryWriteFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.varcharColumnMapping; import static io.trino.plugin.jdbc.StandardColumnMappings.varcharReadFunction; import static io.trino.plugin.jdbc.StandardColumnMappings.varcharWriteFunction; +import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.getUnsupportedTypeHandling; +import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR; import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.connector.JoinCondition.Operator.EQUAL; @@ -127,8 +133,11 @@ import static tech.ydb.trino.YdbColumnMappings.YDB_TIMESTAMP64_SQL_TYPE; import static tech.ydb.trino.YdbColumnMappings.dateColumnMapping; import static tech.ydb.trino.YdbColumnMappings.dateWriteFunction; +import static tech.ydb.trino.YdbColumnMappings.decimalColumnMapping; import static tech.ydb.trino.YdbColumnMappings.timestampColumnMapping; import static tech.ydb.trino.YdbColumnMappings.timestampWriteFunction; +import static tech.ydb.trino.YdbColumnMappings.unsignedBigintColumnMapping; +import static tech.ydb.trino.YdbColumnMappings.unsignedColumnMapping; import static tech.ydb.trino.YdbTableProperties.PRIMARY_KEY_PROPERTY; public class YdbClient extends BaseJdbcClient { @@ -202,6 +211,7 @@ public YdbClient( @Override protected boolean isSupportedJoinCondition(ConnectorSession session, JdbcJoinCondition condition) { return condition.getOperator() == EQUAL + && condition.getLeftColumn().getColumnType().equals(condition.getRightColumn().getColumnType()) && hasSupportedValueMapping(condition.getLeftColumn()) && hasSupportedValueMapping(condition.getRightColumn()); } @@ -213,14 +223,49 @@ private boolean hasSupportedValueMapping(JdbcColumnHandle column) { } String name = type.jdbcTypeName().orElse("").toLowerCase(Locale.ROOT); return switch (name) { - case "bool", "int8", "int16", "int32", "int64", "uint8", "uint16", "uint32", "uint64", - "float", "double", "utf8", "text", "date", "date32", "datetime", "datetime64", - "timestamp", "timestamp64", "decimal" -> true; + case "bool" -> column.getColumnType().equals(BOOLEAN); + case "int8" -> column.getColumnType().equals(TINYINT); + case "int16", "uint8" -> column.getColumnType().equals(SMALLINT); + case "int32", "uint16" -> column.getColumnType().equals(INTEGER); + case "int64", "uint32" -> column.getColumnType().equals(BIGINT); + case "uint64" -> column.getColumnType().equals(createDecimalType(20, 0)); + case "float" -> column.getColumnType().equals(REAL); + case "double" -> column.getColumnType().equals(DOUBLE); + case "utf8", "text" -> column.getColumnType() instanceof VarcharType; + case "date", "date32" -> column.getColumnType().equals(DATE); + case "datetime", "datetime64", "timestamp", "timestamp64" -> column.getColumnType().equals(TIMESTAMP_MICROS); case "string", "bytes" -> column.getColumnType().equals(VARBINARY); - default -> name.startsWith("decimal("); + // Native Decimal also admits NaN/infinities, which Trino DECIMAL cannot represent. + default -> false; }; } + private boolean supportsExpression(ConnectorExpression expression, Map assignments) { + if (expression instanceof Constant constant) { + return YdbTypeUtils.toTypeHandle(constant.getType()).isPresent(); + } + if (expression instanceof Variable variable) { + JdbcColumnHandle column = (JdbcColumnHandle) assignments.get(variable.getName()); + if (!hasSupportedValueMapping(column)) { + return false; + } + return switch (column.getJdbcTypeHandle().jdbcTypeName().orElse("").toLowerCase(Locale.ROOT)) { + case "uint8", "uint16", "uint32", "uint64", "float", "double", + "date", "date32", "datetime", "datetime64", "timestamp" -> false; + default -> true; + }; + } + return expression.getChildren().stream().allMatch(child -> supportsExpression(child, assignments)); + } + + @Override + public boolean supportsAggregationPushdown(ConnectorSession session, JdbcTableHandle table, List aggregates, + Map assignments, List> groupingSets) { + return groupingSets.stream().flatMap(List::stream).map(JdbcColumnHandle.class::cast) + .allMatch(column -> hasSupportedValueMapping(column) + && !column.getColumnType().equals(REAL) && !column.getColumnType().equals(DOUBLE)); + } + @Override public Optional implementJoin( ConnectorSession session, @@ -241,6 +286,19 @@ public Optional implementAggregation( AggregateFunction aggregate, Map assignments ) { + if (aggregate.getArguments().stream().anyMatch(argument -> + argument instanceof Variable variable + ? !hasSupportedValueMapping((JdbcColumnHandle) assignments.get(variable.getName())) + : !supportsExpression(argument, assignments))) { + return Optional.empty(); + } + boolean floatingArgument = aggregate.getArguments().stream().anyMatch(argument -> argument.getType().equals(REAL) || argument.getType().equals(DOUBLE)); + if ((aggregate.getFunctionName().equals("sum") || aggregate.getFunctionName().equals("avg")) && !floatingArgument) { + return Optional.empty(); + } + if (floatingArgument && (aggregate.getFunctionName().equals("min") || aggregate.getFunctionName().equals("max") || aggregate.isDistinct())) { + return Optional.empty(); + } return aggregateFunctionRewriter.rewrite(session, aggregate, assignments); } @@ -250,6 +308,9 @@ public Optional convertPredicate( ConnectorExpression expression, Map assignments ) { + if (!supportsExpression(expression, assignments)) { + return Optional.empty(); + } return connectorExpressionRewriter.rewrite(session, expression, assignments); } @@ -266,6 +327,9 @@ public Optional convertProjection( .anyMatch(column -> column.getColumnName().equals(MERGE_ROW_ID))) { return Optional.empty(); } + if (!supportsExpression(expression, assignments)) { + return Optional.empty(); + } JdbcTypeHandle typeHandle = YdbTypeUtils.toTypeHandle(expression.getType()).orElse(null); if (Objects.isNull(typeHandle)) { @@ -329,6 +393,34 @@ public Optional toColumnMapping( if (jdbcTypeName.equals("utf8") || jdbcTypeName.equals("text")) { return Optional.of(unboundedVarcharColumnMapping()); } + Optional primitiveMapping = switch (jdbcTypeName) { + case "bool" -> Optional.of(booleanColumnMapping()); + case "int8" -> Optional.of(tinyintColumnMapping()); + case "int16" -> Optional.of(smallintColumnMapping()); + case "int32" -> Optional.of(integerColumnMapping()); + case "int64" -> Optional.of(bigintColumnMapping()); + case "uint8" -> Optional.of(unsignedColumnMapping(SMALLINT, 2)); + case "uint16" -> Optional.of(unsignedColumnMapping(INTEGER, 4)); + case "uint32" -> Optional.of(unsignedColumnMapping(BIGINT, 6)); + case "uint64" -> Optional.of(unsignedBigintColumnMapping()); + case "float" -> { + ColumnMapping valueMapping = realColumnMapping(); + yield Optional.of(ColumnMapping.mapping(REAL, valueMapping.getReadFunction(), valueMapping.getWriteFunction(), DISABLE_PUSHDOWN)); + } + case "double" -> { + ColumnMapping valueMapping = doubleColumnMapping(); + yield Optional.of(ColumnMapping.mapping(DOUBLE, valueMapping.getReadFunction(), valueMapping.getWriteFunction(), DISABLE_PUSHDOWN)); + } + case "date", "date32" -> Optional.of(dateColumnMapping(typeHandle)); + case "datetime", "datetime64", "timestamp", "timestamp64" -> Optional.of(timestampColumnMapping(typeHandle)); + default -> Optional.empty(); + }; + if (primitiveMapping.isPresent()) { + return primitiveMapping; + } + if (typeHandle.jdbcType() != Types.DECIMAL) { + return getUnsupportedTypeHandling(session) == CONVERT_TO_VARCHAR ? mapToUnboundedVarchar(typeHandle) : Optional.empty(); + } Optional columnMapping = switch (typeHandle.jdbcType()) { case Types.BIT, Types.BOOLEAN -> Optional.of(booleanColumnMapping()); @@ -434,10 +526,14 @@ public WriteMapping toWriteMapping(ConnectorSession session, Type type) { return WriteMapping.doubleMapping("Double", doubleWriteFunction()); } if (type instanceof DecimalType decimalType) { + if (decimalType.getPrecision() > 35) { + throw new TrinoException(NOT_SUPPORTED, "YDB Decimal precision cannot exceed 35"); + } String dataType = format("Decimal(%s, %s)", decimalType.getPrecision(), decimalType.getScale()); + ColumnMapping mapping = decimalColumnMapping(decimalType); return decimalType.isShort() - ? WriteMapping.longMapping(dataType, shortDecimalWriteFunction(decimalType)) - : WriteMapping.objectMapping(dataType, longDecimalWriteFunction(decimalType)); + ? WriteMapping.longMapping(dataType, (LongWriteFunction) mapping.getWriteFunction()) + : WriteMapping.objectMapping(dataType, (ObjectWriteFunction) mapping.getWriteFunction()); } if (type instanceof VarcharType) { return WriteMapping.sliceMapping("Text", varcharWriteFunction()); @@ -457,7 +553,7 @@ public WriteMapping toWriteMapping(ConnectorSession session, Type type) { @Override public boolean supportsTopN(ConnectorSession session, JdbcTableHandle handle, List sortOrder) { - return true; + return sortOrder.stream().allMatch(item -> hasSupportedValueMapping(item.column())); } @Override @@ -468,19 +564,16 @@ protected Optional topNFunction() { .flatMap(sortItem -> { String columnName = quoted(sortItem.column().getColumnName()); SortOrder sortOrder = sortItem.sortOrder(); - // NULLS FIRST: CASE WHEN col IS NULL THEN 0 ELSE 1 END ASC - // NULLS LAST: CASE WHEN col IS NULL THEN 1 ELSE 0 END ASC - if (sortOrder.isNullsFirst()) { - // Add null-sorting key first, then the actual column - String nullSort = format("CASE WHEN %s IS NULL THEN 0 ELSE 1 END ASC", columnName); - String valueSort = format("%s %s", columnName, sortOrder.isAscending() ? "ASC" : "DESC"); - return Stream.of(nullSort, valueSort); - } else { - // NULLS LAST - String nullSort = format("CASE WHEN %s IS NULL THEN 1 ELSE 0 END ASC", columnName); - String valueSort = format("%s %s", columnName, sortOrder.isAscending() ? "ASC" : "DESC"); - return Stream.of(nullSort, valueSort); + String nullSort = "CASE WHEN %s IS NULL THEN %s ELSE %s END ASC".formatted( + columnName, sortOrder.isNullsFirst() ? 0 : 1, sortOrder.isNullsFirst() ? 1 : 0); + String direction = sortOrder.isAscending() ? "ASC" : "DESC"; + Type type = sortItem.column().getColumnType(); + if (type.equals(REAL) || type.equals(DOUBLE)) { + String nanSort = "CASE WHEN %1$s != %1$s THEN 1 ELSE 0 END %2$s".formatted(columnName, direction); + String yqlType = type.equals(REAL) ? "Float" : "Double"; + return Stream.of(nullSort, nanSort, "NANVL(%s, CAST(0 AS %s)) %s".formatted(columnName, yqlType, direction)); } + return Stream.of(nullSort, columnName + " " + direction); }) .collect(joining(", ")); return format("%s ORDER BY %s LIMIT %d", query, orderBy, limit); diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java index afc4a6cb..b774ddfe 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java @@ -3,17 +3,33 @@ import io.trino.plugin.jdbc.ColumnMapping; import io.trino.plugin.jdbc.JdbcTypeHandle; import io.trino.plugin.jdbc.LongWriteFunction; +import io.trino.plugin.jdbc.ObjectWriteFunction; import io.trino.spi.TrinoException; +import io.trino.spi.type.DecimalType; +import io.trino.spi.type.Int128; +import io.trino.spi.type.Type; +import java.math.BigDecimal; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLDataException; +import java.sql.SQLException; +import java.time.Instant; +import java.time.LocalDateTime; import java.util.Locale; +import static io.trino.plugin.jdbc.PredicatePushdownController.DISABLE_PUSHDOWN; +import static io.trino.plugin.jdbc.PredicatePushdownController.FULL_PUSHDOWN; import static io.trino.plugin.jdbc.StandardColumnMappings.dateReadFunctionUsingLocalDate; import static io.trino.plugin.jdbc.StandardColumnMappings.fromTrinoTimestamp; -import static io.trino.plugin.jdbc.StandardColumnMappings.timestampReadFunction; +import static io.trino.plugin.jdbc.StandardColumnMappings.longDecimalReadFunction; +import static io.trino.plugin.jdbc.StandardColumnMappings.shortDecimalReadFunction; import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; import static io.trino.spi.type.DateType.DATE; +import static io.trino.spi.type.DecimalType.createDecimalType; import static io.trino.spi.type.TimestampType.TIMESTAMP_MICROS; import static java.time.ZoneOffset.UTC; +import static tech.ydb.jdbc.YdbConst.SQL_KIND_DECIMAL; import static tech.ydb.jdbc.YdbConst.SQL_KIND_PRIMITIVE; final class YdbColumnMappings { @@ -37,7 +53,8 @@ static ColumnMapping dateColumnMapping(JdbcTypeHandle typeHandle) { return ColumnMapping.longMapping( DATE, dateReadFunctionUsingLocalDate(), - writeFunction); + writeFunction, + DISABLE_PUSHDOWN); } static ColumnMapping timestampColumnMapping(JdbcTypeHandle typeHandle) { @@ -51,31 +68,112 @@ static ColumnMapping timestampColumnMapping(JdbcTypeHandle typeHandle) { }; return ColumnMapping.longMapping( TIMESTAMP_MICROS, - timestampReadFunction(TIMESTAMP_MICROS), - writeFunction); + (resultSet, index) -> { + Instant instant = typeName.equalsIgnoreCase("Datetime") || typeName.equalsIgnoreCase("Datetime64") + ? resultSet.getObject(index, LocalDateTime.class).toInstant(UTC) + : resultSet.getObject(index, Instant.class); + return Math.addExact(Math.multiplyExact(instant.getEpochSecond(), 1_000_000), instant.getNano() / 1_000); + }, + writeFunction, + typeName.equalsIgnoreCase("Timestamp64") ? FULL_PUSHDOWN : DISABLE_PUSHDOWN); + } + + static ColumnMapping unsignedColumnMapping(Type type, int typeOffset) { + int sqlType = SQL_KIND_PRIMITIVE + typeOffset; + long maximum = switch (typeOffset) { + case 2 -> 255; + case 4 -> 65535; + case 6 -> 0xFFFF_FFFFL; + default -> throw new IllegalArgumentException("Unsupported unsigned type offset: " + typeOffset); + }; + return ColumnMapping.longMapping(type, ResultSet::getLong, + LongWriteFunction.of(sqlType, (statement, index, value) -> { + if (value < 0 || value > maximum) { + throw new SQLDataException("Value is outside the YDB unsigned column range"); + } + statement.setObject(index, value, sqlType); + }), DISABLE_PUSHDOWN); + } + + static ColumnMapping unsignedBigintColumnMapping() { + DecimalType type = createDecimalType(20, 0); + int sqlType = SQL_KIND_PRIMITIVE + 8; + ObjectWriteFunction writer = objectWriteFunction(sqlType, (statement, index, value) -> { + if (value.toBigInteger().signum() < 0 || value.toBigInteger().bitLength() > 64) { + throw new SQLDataException("Value is outside the YDB Uint64 range"); + } + statement.setObject(index, value.toBigInteger().longValue(), sqlType); + }); + return ColumnMapping.objectMapping(type, longDecimalReadFunction(type), writer, DISABLE_PUSHDOWN); + } + + static ColumnMapping decimalColumnMapping(DecimalType type) { + int sqlType = SQL_KIND_DECIMAL + (type.getPrecision() << 6) + type.getScale(); + if (type.isShort()) { + return ColumnMapping.longMapping(type, shortDecimalReadFunction(type), + LongWriteFunction.of(sqlType, (statement, index, value) -> + statement.setObject(index, BigDecimal.valueOf(value, type.getScale()), sqlType)), + DISABLE_PUSHDOWN); + } + return ColumnMapping.objectMapping(type, longDecimalReadFunction(type), + objectWriteFunction(sqlType, (statement, index, value) -> + statement.setObject(index, new BigDecimal(value.toBigInteger(), type.getScale()), sqlType)), + DISABLE_PUSHDOWN); + } + + private static ObjectWriteFunction objectWriteFunction(int sqlType, ObjectWriteFunction.ObjectWriteFunctionImplementation implementation) { + return new ObjectWriteFunction() { + @Override + public Class getJavaType() { + return Int128.class; + } + + @Override + public void set(PreparedStatement statement, int index, Object value) throws SQLException { + implementation.set(statement, index, (Int128) value); + } + + @Override + public void setNull(PreparedStatement statement, int index) throws SQLException { + statement.setNull(index, sqlType); + } + }; } static LongWriteFunction dateWriteFunction(int sqlType) { return LongWriteFunction.of( sqlType, - (statement, index, value) -> statement.setObject(index, value, sqlType)); + (statement, index, value) -> { + if (sqlType == YDB_DATE_SQL_TYPE && (value < 0 || value > 0xFFFFL)) { + throw new SQLDataException("Value is outside the YDB Date storage range"); + } + statement.setObject(index, value, sqlType); + }); } private static LongWriteFunction datetimeWriteFunction(int sqlType) { return LongWriteFunction.of( sqlType, - (statement, index, value) -> statement.setObject( - index, - fromTrinoTimestamp(value).toEpochSecond(UTC), - sqlType)); + (statement, index, value) -> { + if (value % 1_000_000 != 0) { + throw new SQLDataException("YDB Datetime columns cannot store fractional seconds"); + } + long seconds = fromTrinoTimestamp(value).toEpochSecond(UTC); + if (sqlType == YDB_DATETIME_SQL_TYPE && (seconds < 0 || seconds > 0xFFFF_FFFFL)) { + throw new SQLDataException("Value is outside the YDB Datetime storage range"); + } + statement.setObject(index, seconds, sqlType); + }); } static LongWriteFunction timestampWriteFunction(int sqlType) { return LongWriteFunction.of( sqlType, - (statement, index, value) -> statement.setObject( - index, - fromTrinoTimestamp(value).toInstant(UTC), - sqlType)); + (statement, index, value) -> { + if (sqlType == YDB_TIMESTAMP_SQL_TYPE && value < 0) { + throw new SQLDataException("Value is outside the YDB Timestamp storage range"); + } + statement.setObject(index, fromTrinoTimestamp(value).toInstant(UTC), sqlType); + }); } } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java index 6ab88d69..22eabeac 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbQueryBuilder.java @@ -28,7 +28,7 @@ protected String formatJoinCondition(JdbcClient client, String leftAlias, String private String formatJoinKey(JdbcClient client, String alias, JdbcColumnHandle column) { String reference = alias + "." + client.quoted(column.getColumnName()); if (column.getJdbcTypeHandle().jdbcTypeName().filter("Uint64"::equalsIgnoreCase).isPresent()) { - return "BITCAST(" + reference + " AS Int64)"; + return "CAST(" + reference + " AS Decimal(20, 0))"; } if (column.getColumnType().equals(REAL) || column.getColumnType().equals(DOUBLE)) { // Match Trino's JOIN behavior for NaN and signed zero. diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbTypeUtils.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbTypeUtils.java index 95f737b7..9b194e6c 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbTypeUtils.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbTypeUtils.java @@ -1,46 +1,55 @@ package tech.ydb.trino; import io.trino.plugin.jdbc.JdbcTypeHandle; +import io.trino.spi.type.BigintType; +import io.trino.spi.type.BooleanType; +import io.trino.spi.type.DateType; import io.trino.spi.type.DecimalType; +import io.trino.spi.type.DoubleType; +import io.trino.spi.type.IntegerType; +import io.trino.spi.type.RealType; +import io.trino.spi.type.SmallintType; +import io.trino.spi.type.TimestampType; +import io.trino.spi.type.TinyintType; import io.trino.spi.type.Type; +import io.trino.spi.type.VarbinaryType; +import io.trino.spi.type.VarcharType; import java.sql.Types; import java.util.Optional; public final class YdbTypeUtils { - private YdbTypeUtils() { - - } + private YdbTypeUtils() {} public static Optional toTypeHandle(Type type) { return switch (type) { - case io.trino.spi.type.BooleanType _ -> + case BooleanType _ -> Optional.of(new JdbcTypeHandle(Types.BOOLEAN, Optional.of("Bool"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.TinyintType _ -> + case TinyintType _ -> Optional.of(new JdbcTypeHandle(Types.TINYINT, Optional.of("Int8"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.SmallintType _ -> + case SmallintType _ -> Optional.of(new JdbcTypeHandle(Types.SMALLINT, Optional.of("Int16"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.IntegerType _ -> + case IntegerType _ -> Optional.of(new JdbcTypeHandle(Types.INTEGER, Optional.of("Int32"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.BigintType _ -> + case BigintType _ -> Optional.of(new JdbcTypeHandle(Types.BIGINT, Optional.of("Int64"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.RealType _ -> + case RealType _ -> Optional.of(new JdbcTypeHandle(Types.REAL, Optional.of("Float"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.DoubleType _ -> + case DoubleType _ -> Optional.of(new JdbcTypeHandle(Types.DOUBLE, Optional.of("Double"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.DateType _ -> - Optional.of(new JdbcTypeHandle(Types.DATE, Optional.of("Date"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.TimestampType _ -> - Optional.of(new JdbcTypeHandle(Types.TIMESTAMP, Optional.of("Timestamp"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case DecimalType decimalType -> + case DateType _ -> + Optional.of(new JdbcTypeHandle(Types.DATE, Optional.of("Date32"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); + case TimestampType timestampType when timestampType.getPrecision() == 3 || timestampType.getPrecision() == 6 -> + Optional.of(new JdbcTypeHandle(Types.TIMESTAMP, Optional.of("Timestamp64"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); + case DecimalType decimalType when decimalType.getPrecision() <= 35 -> Optional.of(new JdbcTypeHandle(Types.DECIMAL, Optional.of("Decimal"), Optional.of(decimalType.getPrecision()), Optional.of(decimalType.getScale()), Optional.empty(), Optional.empty())); - case io.trino.spi.type.VarcharType _ -> + case VarcharType _ -> Optional.of(new JdbcTypeHandle(Types.VARCHAR, Optional.of("Text"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); - case io.trino.spi.type.VarbinaryType _ -> + case VarbinaryType _ -> Optional.of(new JdbcTypeHandle(Types.BINARY, Optional.of("Bytes"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); default -> - Optional.of(new JdbcTypeHandle(Types.VARCHAR, Optional.of("String"), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty())); + Optional.empty(); }; } } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java new file mode 100644 index 00000000..2a714c15 --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java @@ -0,0 +1,135 @@ +package tech.ydb.trino; + +import io.trino.plugin.base.mapping.DefaultIdentifierMapping; +import io.trino.plugin.jdbc.BaseJdbcConfig; +import io.trino.plugin.jdbc.ColumnMapping; +import io.trino.plugin.jdbc.JdbcTypeHandle; +import io.trino.plugin.jdbc.LongReadFunction; +import io.trino.plugin.jdbc.LongWriteFunction; +import io.trino.plugin.jdbc.ObjectReadFunction; +import io.trino.plugin.jdbc.ObjectWriteFunction; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.type.Int128; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Proxy; +import java.math.BigDecimal; +import java.math.BigInteger; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.SQLDataException; +import java.sql.SQLException; +import java.sql.Types; +import java.time.Instant; +import java.time.LocalDateTime; +import java.util.ArrayList; +import java.util.Arrays; +import java.util.List; +import java.util.Optional; + +import static io.trino.spi.type.DecimalType.createDecimalType; +import static io.trino.spi.type.SmallintType.SMALLINT; +import static io.trino.testing.TestingConnectorSession.SESSION; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; +import static tech.ydb.jdbc.YdbConst.SQL_KIND_DECIMAL; +import static tech.ydb.jdbc.YdbConst.SQL_KIND_PRIMITIVE; + +public class TestYdbColumnMappings { + private final YdbClient client = new YdbClient( + new BaseJdbcConfig(), + _ -> { + throw new SQLException("This test must not open a connection"); + }, + new YdbQueryBuilder(RemoteQueryModifier.NONE), + new DefaultIdentifierMapping(), + RemoteQueryModifier.NONE); + + @Test + public void testUnsignedBigintRoundTrip() throws Exception { + JdbcTypeHandle handle = new JdbcTypeHandle(Types.BIGINT, Optional.of("Uint64"), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + ColumnMapping mapping = client.toColumnMapping(SESSION, null, handle).orElseThrow(); + assertThat(mapping.getType()).isEqualTo(createDecimalType(20, 0)); + BigInteger maximum = new BigInteger("18446744073709551615"); + ResultSet resultSet = (ResultSet) Proxy.newProxyInstance(ResultSet.class.getClassLoader(), new Class[]{ResultSet.class}, + (_, method, _) -> { + assertThat(method.getName()).isEqualTo("getBigDecimal"); + return new BigDecimal(maximum); + }); + Int128 value = (Int128) ((ObjectReadFunction) mapping.getReadFunction()).readObject(resultSet, 1); + assertThat(value.toBigInteger()).isEqualTo(maximum); + + List> calls = new ArrayList<>(); + PreparedStatement statement = statement(calls); + ObjectWriteFunction writer = (ObjectWriteFunction) mapping.getWriteFunction(); + writer.set(statement, 1, value); + writer.setNull(statement, 2); + assertThat(calls).containsExactly(List.of(1, -1L, SQL_KIND_PRIMITIVE + 8), List.of(2, SQL_KIND_PRIMITIVE + 8)); + assertThatThrownBy(() -> writer.set(statement, 1, Int128.valueOf(-1))).isInstanceOf(SQLDataException.class); + assertThatThrownBy(() -> writer.set(statement, 1, Int128.valueOf(BigInteger.ONE.shiftLeft(64)))).isInstanceOf(SQLDataException.class); + } + + @Test + public void testUnsignedWriterBounds() throws Exception { + List> calls = new ArrayList<>(); + PreparedStatement statement = statement(calls); + LongWriteFunction writer = (LongWriteFunction) YdbColumnMappings.unsignedColumnMapping(SMALLINT, 2).getWriteFunction(); + writer.set(statement, 1, 255); + assertThat(calls).containsExactly(List.of(1, 255L, SQL_KIND_PRIMITIVE + 2)); + assertThatThrownBy(() -> writer.set(statement, 1, -1)).isInstanceOf(SQLDataException.class); + assertThatThrownBy(() -> writer.set(statement, 1, 256)).isInstanceOf(SQLDataException.class); + } + + @Test + public void testDecimalWriterPreservesPrecisionAndScale() throws Exception { + List> calls = new ArrayList<>(); + PreparedStatement statement = statement(calls); + LongWriteFunction shortWriter = (LongWriteFunction) YdbColumnMappings.decimalColumnMapping(createDecimalType(10, 3)).getWriteFunction(); + shortWriter.set(statement, 1, 12345); + ObjectWriteFunction longWriter = (ObjectWriteFunction) YdbColumnMappings.decimalColumnMapping(createDecimalType(35, 10)).getWriteFunction(); + longWriter.set(statement, 2, Int128.valueOf(new BigInteger("123456789012345678901234567890"))); + longWriter.setNull(statement, 3); + assertThat(calls).containsExactly( + List.of(1, new BigDecimal("12.345"), SQL_KIND_DECIMAL + (10 << 6) + 3), + List.of(2, new BigDecimal("12345678901234567890.1234567890"), SQL_KIND_DECIMAL + (35 << 6) + 10), + List.of(3, SQL_KIND_DECIMAL + (35 << 6) + 10)); + } + + @Test + public void testUtcTimestampReads() throws Exception { + for (String type : List.of("Datetime", "Datetime64", "Timestamp", "Timestamp64")) { + ResultSet resultSet = (ResultSet) Proxy.newProxyInstance(ResultSet.class.getClassLoader(), new Class[]{ResultSet.class}, + (_, method, args) -> { + assertThat(method.getName()).isEqualTo("getObject"); + if (type.startsWith("Datetime")) { + assertThat(args[1]).isEqualTo(LocalDateTime.class); + return LocalDateTime.parse("2020-02-29T12:34:56"); + } + assertThat(args[1]).isEqualTo(Instant.class); + return Instant.parse("2020-02-29T12:34:56Z"); + }); + JdbcTypeHandle handle = new JdbcTypeHandle(Types.TIMESTAMP, Optional.of(type), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + LongReadFunction reader = (LongReadFunction) YdbColumnMappings.timestampColumnMapping(handle).getReadFunction(); + assertThat(reader.readLong(resultSet, 1)).isEqualTo(Instant.parse("2020-02-29T12:34:56Z").getEpochSecond() * 1_000_000); + } + } + + @Test + public void testDatetimeRejectsFractionalSeconds() { + JdbcTypeHandle handle = new JdbcTypeHandle(Types.TIMESTAMP, Optional.of("Datetime64"), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + LongWriteFunction writer = (LongWriteFunction) YdbColumnMappings.timestampColumnMapping(handle).getWriteFunction(); + assertThatThrownBy(() -> writer.set(statement(new ArrayList<>()), 1, 1)).isInstanceOf(SQLDataException.class); + } + + private static PreparedStatement statement(List> calls) { + return (PreparedStatement) Proxy.newProxyInstance(PreparedStatement.class.getClassLoader(), new Class[]{PreparedStatement.class}, + (_, method, args) -> { + assertThat(method.getName()).isIn("setObject", "setNull"); + calls.add(Arrays.asList(args)); + return null; + }); + } +} diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java new file mode 100644 index 00000000..35d4e794 --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java @@ -0,0 +1,88 @@ +package tech.ydb.trino; + +import io.trino.plugin.base.mapping.DefaultIdentifierMapping; +import io.trino.plugin.jdbc.BaseJdbcConfig; +import io.trino.plugin.jdbc.JdbcColumnHandle; +import io.trino.plugin.jdbc.JdbcJoinCondition; +import io.trino.plugin.jdbc.JdbcSortItem; +import io.trino.plugin.jdbc.JdbcTypeHandle; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.type.ArrayType; +import io.trino.spi.type.Type; +import org.junit.jupiter.api.Test; + +import java.sql.SQLException; +import java.sql.Types; +import java.util.List; +import java.util.Optional; + +import static io.trino.spi.connector.JoinCondition.Operator.EQUAL; +import static io.trino.spi.connector.JoinCondition.Operator.IDENTICAL; +import static io.trino.spi.connector.JoinCondition.Operator.LESS_THAN; +import static io.trino.spi.connector.SortOrder.ASC_NULLS_LAST; +import static io.trino.spi.type.BigintType.BIGINT; +import static io.trino.spi.type.DecimalType.createDecimalType; +import static io.trino.spi.type.DoubleType.DOUBLE; +import static io.trino.spi.type.RealType.REAL; +import static io.trino.spi.type.TimestampType.createTimestampType; +import static io.trino.testing.TestingConnectorSession.SESSION; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestYdbJoinMappings { + private final YdbClient client = new YdbClient( + new BaseJdbcConfig(), + _ -> { + throw new SQLException("This test must not open a connection"); + }, + new YdbQueryBuilder(RemoteQueryModifier.NONE), + new DefaultIdentifierMapping(), + RemoteQueryModifier.NONE); + + @Test + public void testSupportedNativeEqualityKeys() { + for (String name : List.of("Bool", "Int8", "Int16", "Int32", "Int64", "Uint8", "Uint16", "Uint32", "Uint64", + "Float", "Double", "Utf8", "String", "Date", "Date32", "Datetime", "Datetime64", "Timestamp", "Timestamp64")) { + JdbcTypeHandle handle = typeHandle(name); + Type type = client.toColumnMapping(SESSION, null, handle).orElseThrow().getType(); + JdbcColumnHandle left = new JdbcColumnHandle("left", handle, type); + JdbcColumnHandle right = new JdbcColumnHandle("right", handle, type); + assertThat(client.isSupportedJoinCondition(SESSION, new JdbcJoinCondition(left, EQUAL, right))).as(name).isTrue(); + assertThat(client.isSupportedJoinCondition(SESSION, new JdbcJoinCondition(left, LESS_THAN, right))).as(name).isFalse(); + assertThat(client.isSupportedJoinCondition(SESSION, new JdbcJoinCondition(left, IDENTICAL, right))).as(name).isFalse(); + } + } + + @Test + public void testDecimalAndMixedTypesFallBack() { + JdbcColumnHandle decimal = new JdbcColumnHandle("decimal", typeHandle("Decimal(20,0)"), createDecimalType(20, 0)); + JdbcColumnHandle unsigned = new JdbcColumnHandle("unsigned", typeHandle("Uint64"), createDecimalType(20, 0)); + JdbcColumnHandle signed = new JdbcColumnHandle("signed", typeHandle("Int64"), BIGINT); + assertThat(client.isSupportedJoinCondition(SESSION, new JdbcJoinCondition(decimal, EQUAL, decimal))).isFalse(); + assertThat(client.isSupportedJoinCondition(SESSION, new JdbcJoinCondition(unsigned, EQUAL, decimal))).isFalse(); + assertThat(client.isSupportedJoinCondition(SESSION, new JdbcJoinCondition(unsigned, EQUAL, signed))).isFalse(); + } + + @Test + public void testFloatingPointNormalizationAndTypedTopN() { + YdbQueryBuilder queryBuilder = new YdbQueryBuilder(RemoteQueryModifier.NONE); + for (Type type : List.of(REAL, DOUBLE)) { + String nativeType = type.equals(REAL) ? "Float" : "Double"; + JdbcColumnHandle column = new JdbcColumnHandle("key", typeHandle(nativeType), type); + String condition = queryBuilder.formatJoinCondition(client, "l", "r", new JdbcJoinCondition(column, EQUAL, column)); + assertThat(condition).contains("NANVL(", "CAST(NULL AS " + nativeType + ")", "l.`key`", "r.`key`"); + String topN = client.topNFunction().orElseThrow().apply("SELECT `key` FROM `test`", + List.of(new JdbcSortItem(column, ASC_NULLS_LAST)), 2); + assertThat(topN).contains("NANVL(`key`, CAST(0 AS " + nativeType + ")) ASC"); + } + } + + @Test + public void testUnsupportedProjectionTypesHaveNoMapping() { + assertThat(YdbTypeUtils.toTypeHandle(new ArrayType(BIGINT))).isEmpty(); + assertThat(YdbTypeUtils.toTypeHandle(createTimestampType(12))).isEmpty(); + } + + private static JdbcTypeHandle typeHandle(String name) { + return new JdbcTypeHandle(Types.OTHER, Optional.of(name), Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + } +} From e2d9931079a5e9033c1d57de13d5344454387e75 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 17:43:41 +0300 Subject: [PATCH 07/13] Preserve native types and resource ownership in YDB writes 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. --- ydb-trino-adapter/README.md | 34 ++- ydb-trino-adapter/ROADMAP.md | 48 ++-- ydb-trino-adapter/UPSTREAM-READINESS.md | 103 ++++++++ ydb-trino-adapter/pom.xml | 12 +- .../trino/RewriteUnaryStringOperations.java | 4 +- .../main/java/tech/ydb/trino/YdbClient.java | 51 +++- .../java/tech/ydb/trino/YdbClientModule.java | 5 + .../java/tech/ydb/trino/YdbConnector.java | 7 +- .../tech/ydb/trino/YdbConnectorFactory.java | 3 + .../java/tech/ydb/trino/YdbMergeSink.java | 236 ++++++++++++++++++ .../tech/ydb/trino/YdbPageSinkProvider.java | 40 +++ .../ydb/trino/TestYdbConnectorSmokeTest.java | 4 +- .../tech/ydb/trino/TestYdbConnectorTest.java | 6 +- .../tech/ydb/trino/TestYdbCreateTable.java | 102 +++++++- .../ydb/trino/TestYdbExpressionRewrites.java | 9 + .../tech/ydb/trino/TestYdbJoinPushdown.java | 164 ++++++++++++ .../java/tech/ydb/trino/TestYdbMergeSink.java | 217 ++++++++++++++++ .../tech/ydb/trino/TestYdbWriteMetadata.java | 74 ++++++ .../tech/ydb/trino/TestingYdbJdbcModule.java | 5 + .../java/tech/ydb/trino/YdbQueryRunner.java | 11 +- 20 files changed, 1091 insertions(+), 44 deletions(-) create mode 100644 ydb-trino-adapter/UPSTREAM-READINESS.md create mode 100644 ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbMergeSink.java create mode 100644 ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPageSinkProvider.java create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbMergeSink.java create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbWriteMetadata.java diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index 2cadb69f..94984cb2 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -60,9 +60,32 @@ AS SELECT tenant, event_id, payload FROM events; ``` Имена ключей в CTAS относятся к выходным столбцам запроса. Отсутствующий, -пустой, повторяющийся или неизвестный `primary_key` отклоняется. Текущая -integration-проверка CTAS использует `insert.non-transactional-insert.enabled=true`; -transactional staging этой проверкой не подтверждается. +пустой, повторяющийся или неизвестный `primary_key` отклоняется. +Обычный INSERT использует временную таблицу с native-типами и отдельным +Serial-ключом; публикация в целевую таблицу выполняется одним `INSERT ... SELECT`. +`TestYdbCreateTable` проверяет этот режим на production client, включая ошибку +повторяющегося ключа и очистку временных таблиц. + +## UPDATE, DELETE и MERGE + +Полностью передаваемые в YDB UPDATE/DELETE возвращают число строк через YQL +`RETURNING` в том же запросе. Изменение физического первичного ключа явно +отклоняется с `NOT_SUPPORTED`: [ограничение YQL UPDATE](https://ydb.tech/docs/en/yql/reference/syntax/update). + +Для MERGE и UPDATE/DELETE, которые Trino выполняет построчно, нужен явный режим: + +```properties +merge.non-transactional-merge.enabled=true +``` + +Либо `SET SESSION local.non_transactional_merge = true`. Такой запрос +**не атомарен целиком**: уже завершённые пакеты остаются после ошибки или отмены. +Каждый пакет размером не более `write.batch-size` использует собственные +соединение и транзакцию; операции сохраняют порядок, nullable-составной ключ +сопоставляется без потери NULL, а запись сохраняет native-типы. +Ошибка откатывает текущий пакет; соединения и statements закрываются до +возврата из обработки пакета. Автоматического replay нет, query/task retry запрещён. +Ограничение числа writer tasks в Trino 483 не обеспечивает атомарность MERGE. ## JOIN pushdown @@ -98,6 +121,7 @@ Native decimal `NaN` и бесконечности не представимы передаётся в YDB только для целочисленного результата с ненулевым постоянным делителем, отличным от `-1`. `strpos` считает позиции Unicode-символов и сохраняет SQL `NULL`, а не заменяет его нулём. +`trim` остаётся в Trino: byte-oriented `String::Strip` не заменяет Unicode trim. YDB `Text` отображается в Trino как `varchar`, а `Bytes` — как `varbinary` без декодирования UTF-8. При создании таблиц адаптер использует типы `Text` и `Bytes`. @@ -113,7 +137,7 @@ YDB `Text` отображается в Trino как `varchar`, а `Bytes` — к в `Datetime`/`Datetime64` отклоняется без молчаливого усечения. При записи адаптер передаёт YDB JDBC точный vendor type по `TYPE_NAME` и больше не задаёт `forceSignedDatetimes`: `Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp`/`Timestamp64` — `Instant` с микросекундами. -В `UPDATE`/`DELETE`, которые стандартный merge sink выполняет внутри `MERGE`, native `TYPE_NAME` не передаётся; -fallback `Instant` для `Datetime`/`Datetime64` может зависеть от часового пояса JVM и остаётся отдельным риском. +В построчных UPDATE/DELETE собственный merge sink использует исходный +`JdbcTypeHandle` и те же native write mappings, что и обычная запись. Предикаты для `Date`/`Date32` и legacy temporal типов оставляются в Trino, чтобы не связывать значения вне диапазона исходного YDB-типа. diff --git a/ydb-trino-adapter/ROADMAP.md b/ydb-trino-adapter/ROADMAP.md index eacce7c9..334314b1 100644 --- a/ydb-trino-adapter/ROADMAP.md +++ b/ydb-trino-adapter/ROADMAP.md @@ -44,14 +44,14 @@ capability. Trino 483 removes the inherited `testDropTableIfExists`, `testSymbolAliasing` methods, plus the duplicate smoke-test information-schema method. -Validation completed in this checkout with Temurin 25.0.2: +Historical 479-to-483 migration validation with Temurin 25.0.2: - clean test compilation: 16 production sources and 5 test sources compiled; - package with tests skipped: successful; - dependency audit: every Trino release-coupled artifact resolves to 483; - Docker-backed tests: 0 completed. The existing Colima default profile is - broken, so focused and full-suite counts are not yet available and no upgrade - PR may be created. + unavailable during that run. These historical counts do not validate the + subsequent changes below. ## Pre-upgrade DML baseline (Trino 479) @@ -74,28 +74,30 @@ within the CI budget. An isolated local Colima run previously exceeded 11 minutes, so MERGE scalability remains a production concern rather than a CI failure. -## P0 — harden standard MERGE execution - -1. Keep MERGE on Trino's `JdbcMergeSink`; YDB-specific code should only build - the handle and supply primary-key metadata. -2. The standard sink may commit INSERT, DELETE, and UPDATE batches through - separate JDBC connections. Keep one writer task and reject Trino query/task - retries, but do not claim that this makes MERGE atomic across operation sinks. -3. YQL `UPDATE` cannot change a primary-key value. Implement physical-key - changes as atomic delete+insert row changes, or reject that statement with a - documented `NOT_SUPPORTED` error. See the - [YQL UPDATE contract](https://ydb.tech/docs/en/yql/reference/syntax/update). -4. Add focused tests for composite primary keys, a non-unique first visible - column, physical-key updates, partial sink failures, and abort cleanup. - -**Exit criterion:** retain the current green inherited `testMerge*` suite and -cover partial failures between operation sinks. If atomic MERGE becomes a -requirement, implement staging plus one finalize transaction instead of -wrapping the standard sink in replay logic. +## MERGE contract after connector review + +`JdbcMergeSink` discards native metadata in its operation-specific handles and +retains connections until finish/abort. Trino 483 does not call merge-sink abort +from operator close and does not enforce the connector's writer limit for MERGE. +A single-writer setting cannot establish statement atomicity. + +The connector therefore requires explicit `non_transactional_merge=true` for +row-level writes. `YdbMergeSink` uses native mappings and bounded, owned batch +transactions, preserves operation and composite-key order, handles NULL keys, +and closes all resources before returning. Earlier successful batches remain +committed on failure; no replay is attempted. Physical-key changes fail with +`NOT_SUPPORTED` before execution, as required by the +[YQL UPDATE contract](https://ydb.tech/docs/en/yql/reference/syntax/update). + +Unit tests cover order, nullable keys, rollback, partial batch failure and +resource closure; production-client tests cover composite-key MERGE and default +INSERT staging. Full inherited tests must also pass before a release claim. +Statement-atomic MERGE remains future work, requiring coordinator finalization +or verified upstream planner and cancellation changes, not an implicit promise. ## P1 — retry and transaction hardening -- Do not wrap `JdbcMergeSink` in connector-owned replay after any driver or YDB +- Do not wrap a merge sink in connector-owned replay after any driver or YDB failure. A future retry design requires staging or an operation ID that proves replay safety. See [YDB SDK error handling](https://ydb.tech/docs/en/reference/ydb-sdk/error_handling). @@ -156,7 +158,7 @@ that inherit from this behavior and also remain false. See YDB - List/Dict/Struct mappings for Trino ARRAY/MAP/ROW; - views, comments, rename column, and type changes after checking current YQL semantics; -- transactional INSERT/staging instead of direct non-transactional writes; +- complete real-YDB validation of default transactional INSERT staging; - complete the Trino 483 default-column behavior group before advertising it. ## Validation ladder diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md new file mode 100644 index 00000000..4910849a --- /dev/null +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -0,0 +1,103 @@ +# Connector review and upstream handoff + +This change is delivered to `ydb-platform/ydb-java-dialects`, not to Trino. +The standalone module remains pinned to Trino 483. The upstream comparison +and prototype used Trino `484-SNAPSHOT` at +`8d58343762f56a7d0f13b799f5521ddca6fbcfff`, with Java 25. +Binary or source compatibility between these versions is not assumed. + +## Comparison with the pinned Trino tree + +| Area | Trino reference | YDB decision | +| --- | --- | --- | +| Packaging | `plugin/trino-postgresql/pom.xml`, root POM, server assembly | An upstream module needs parent-managed `trino-plugin` packaging, root module and server assembly registration, provided SPI dependencies, and generated plugin services. The standalone shaded-driver POM is not an upstream module. | +| SPI | `lib/trino-plugin-toolkit/.../Versions.java` and JDBC sink interfaces | Enforce the compiled SPI version. The 484 provider/sink memory-context signatures differ from 483 and require an explicit port. | +| JOIN | PostgreSQL, MySQL and MariaDB JDBC clients; `DefaultQueryBuilder` | Use framework join plumbing, but a YQL-specific scalar allowlist and key normalization. Do not inherit another database's comparison semantics. | +| Expressions | `ConnectorExpressionRewriter`, JDBC rewrite rules | Preserve parameter occurrence order. Leave overflow-sensitive arithmetic, Unicode trim and unsupported native coercions in Trino. | +| Types | `StandardColumnMappings`, pinned YDB JDBC `MappingGetters`/`YdbTypes` | Use native unsigned and temporal mappings. Uint64 becomes decimal(20,0), not a signed bigint. Decimal writes carry precision and scale. | +| Metadata | `BaseJdbcClient.getColumns` and YDB JDBC metadata | YDB reports neither TABLE_CAT nor TABLE_SCHEM. The virtual `default` schema must not become a metadata row filter or a physical directory. Staging uses the driver's actual identity. | +| Writes | `JdbcPageSink`, `JdbcMergeSink`, `BaseJdbcClient.beginMerge` | Retain ordinary JDBC INSERT plumbing with YDB schema-copy DDL. Row-level writes explicitly require non-transactional mode and use bounded native-typed batches, rather than operation-specific sinks with lost native metadata. | +| Counts | `QueryBuilder.prepareUpdateQuery`/`prepareDeleteQuery` | Pushdown UPDATE/DELETE uses YQL RETURNING, not a preceding COUNT. Row-level counts follow Trino's processed-row contract in the explicitly non-transactional mode. | +| Tests | `BaseConnectorTest`, `BaseConnectorSmokeTest` | Keep inherited capability tests. Add a production-client native JOIN matrix and write tests; the test-only hidden-key client does not establish production CREATE/INSERT semantics. | +| Documentation/CI | `.github/CONTRIBUTING.md`, `.github/DEVELOPMENT.md`, Trino connector docs and CI | An upstream submission also requires documentation registration, formatting, dependency checks, fresh Error Prone, the ordinary CI matrix and CLA. Standalone Maven success does not establish these. | + +## Correctness changes + +- Equality JOIN covers Bool, signed/unsigned integers, floating point, text, + bytes and temporal native types, with INNER/LEFT/RIGHT/FULL test cases. + Uint64 comparisons retain the full range. NaN and signed zero are normalized + for ordinary equality; null-safe equality, native Decimal and unsafe + expressions remain in Trino. +- NULLIF and Unicode strpos preserve repeated bind parameters and SQL NULL. + Integral overflow is not silently converted into YQL NULL or wrapping. +- Native numeric and temporal writers preserve type, range and precision. + Floating Top-N uses typed NANVL arguments and explicit NULL/NaN ordering. +- INSERT staging copies native columns without confusing Trino's virtual + schema with JDBC metadata. A separate Serial key avoids reusing the target + key as staging row identity. +- Physical-key updates fail explicitly before DML. MERGE retains key order, + handles nullable composite keys and closes each owned batch transaction. + Failed batches roll back without replay; previous batches can remain. +- Docker unavailability is an integration-test failure, not a silent skip. + Shared YDB helper use is serialized. CTAS cleanup assertions subscribe to + the actual rollback completion instead of racing asynchronous cleanup. + +## Upstream prototype evidence and limitations + +The mistakenly opened `trinodb/trino#31384` is closed. Its CI reported +113 successful checks, 5 failures and 2 cancellations. Failures included +connector tests, fresh Error Prone, commit-message formatting, CLA, and the +aggregate gate. It is not evidence of upstream readiness. + +That Linux run did exercise real YDB: the native JOIN class had 25 test +entries, 1 failure and 1 error. The failing cases exposed an overflow assertion +and untyped floating NANVL argument, both addressed in the standalone tests +and implementation. INSERT metadata identity and MERGE failures also required +changes; the old integration result must not be reused as a pass for this tree. + +An independent review of prototype commit +`044a7a0646a2271623c854618482f2c94f972dcc` rejected its atomic-MERGE claim: +one writer was not actually enforced, cancellation did not abort the sink, +and staging metadata still used an inconsistent identity. + +The separate local Trino checkout preserves follow-up commits: + +- `cf6e4cf33f`: abort unfinished MERGE sinks on operator close; +- `bd9a654ce3`: correct INSERT staging metadata identity; +- `dcb449c1f3`: enforce connector limits for unpartitioned MERGE writers. + +Its affected core planner/operator group passed 21 tests. Those engine changes +are not part of Trino 483 and are not silently assumed here. The current +standalone implementation deliberately makes only a non-transactional MERGE +claim. Statement-atomic MERGE needs additional design and verification. + +The local Trino checkout is +`/Users/kurdyukov-kir/IdeaProjects/ydb-upstream-work/trino`, branch +`add-ydb-connector`. The upstream submission belongs to the student's handoff; +no replacement upstream PR is created by this work. + +## Validation + +With Temurin 25.0.2, the following standalone command passed 18 tests, with +zero failures, errors or skips, and built the module: + +```bash +mvn -B -ntp -f ydb-trino-adapter/pom.xml \ + -Dtest=TestYdbExpressionRewrites,TestYdbColumnMappings,TestYdbJoinMappings,TestYdbMergeSink,TestYdbWriteMetadata \ + verify +``` + +The local run used a separate Maven repository via `-Dmaven.repo.local`. +Java LSP diagnostics were unavailable because jdtls is not installed. +Docker's Colima socket was unavailable; Docker/Colima was neither restarted +nor repaired. The local full `clean test` attempted 22 entries: 18 passed, +4 integration classes failed in helper setup, with zero errors or skips. +No local integration test completed. Full integration results must come from +the ordinary module CI: + +```bash +mvn -B -ntp -f ydb-trino-adapter/pom.xml clean test +``` + +The current PR's exact-head CI and independent review, not the historical +prototype or earlier PRs, determine whether this branch is ready to merge. diff --git a/ydb-trino-adapter/pom.xml b/ydb-trino-adapter/pom.xml index 751130cb..09d5ac03 100644 --- a/ydb-trino-adapter/pom.xml +++ b/ydb-trino-adapter/pom.xml @@ -18,7 +18,7 @@ 25 483 2.4.1 - 2.3.13 + 2.4.10 5.10.1 @@ -64,6 +64,12 @@ ${junit.version} test + + org.junit.jupiter + junit-jupiter-params + ${junit.version} + test + tech.ydb.test ydb-junit5-support @@ -155,6 +161,10 @@ 3.1.0 --add-modules jdk.incubator.vector + + ydbplatform/local-ydb@sha256:9e46fd45875551a75bcf34d0bb9ca0baa1d8763a4ccf2070af45f4467c4b7402 + false + diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteUnaryStringOperations.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteUnaryStringOperations.java index 5f4536e6..a8339420 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteUnaryStringOperations.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteUnaryStringOperations.java @@ -29,8 +29,7 @@ public class RewriteUnaryStringOperations implements ProjectFunctionRule VALUE = newCapture(); private static final Pattern PATTERN = call() - .with(functionName().matching(name -> name.equals(new FunctionName("trim")) || - name.equals(new FunctionName("upper")) || + .with(functionName().matching(name -> name.equals(new FunctionName("upper")) || name.equals(new FunctionName("lower")))) .with(type().matching(type -> type instanceof VarcharType)) .with(argumentCount().equalTo(1)) @@ -75,7 +74,6 @@ private static String mapToYql(String functionName) { return switch (functionName) { case "upper" -> "Unicode::ToUpper(%s)"; case "lower" -> "Unicode::ToLower(%s)"; - case "trim" -> "String::Strip(%s)"; default -> throw new IllegalArgumentException("Unexpected function name: " + functionName); }; } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index 26016ad5..912c65f5 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -85,6 +85,7 @@ import static io.trino.plugin.jdbc.DefaultJdbcMetadata.MERGE_ROW_ID; import static io.trino.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; +import static io.trino.plugin.jdbc.JdbcWriteSessionProperties.isNonTransactionalMerge; import static io.trino.plugin.jdbc.PredicatePushdownController.DISABLE_PUSHDOWN; import static io.trino.plugin.jdbc.PredicatePushdownController.FULL_PUSHDOWN; import static io.trino.plugin.jdbc.StandardColumnMappings.bigintColumnMapping; @@ -657,7 +658,7 @@ protected String getColumnDefinitionSql(ConnectorSession session, ColumnMetadata sb.append(" NOT NULL"); } if (column.getDefaultValue().isPresent()) { - sb.append(" DEFAULT ").append(column.getDefaultValue().get()); + throw new TrinoException(NOT_SUPPORTED, "This connector does not support defining column defaults through Trino"); } return sb.toString(); @@ -741,6 +742,35 @@ public boolean supportsMerge() { return true; } + @Override + protected void copyTableSchema(ConnectorSession session, Connection connection, String catalogName, + String schemaName, String tableName, String newTableName, List columnNames) { + // YDB JDBC reports neither TABLE_CAT nor TABLE_SCHEM for discovered tables. + RemoteTableName remoteTable = new RemoteTableName(Optional.empty(), Optional.empty(), tableName); + Map sourceColumns = getColumns(session, new SchemaTableName(DEFAULT_SCHEMA, tableName), remoteTable).stream() + .collect(Collectors.toMap(JdbcColumnHandle::getColumnName, Function.identity())); + String stagingKey = "_trino_ydb_staging_key"; + while (columnNames.contains(stagingKey)) { + stagingKey += "_"; + } + String declarations = columnNames.stream().map(name -> { + JdbcColumnHandle column = sourceColumns.get(name); + String nativeType = column.getJdbcTypeHandle().jdbcTypeName().orElseThrow(); + if (nativeType.equalsIgnoreCase("Decimal")) { + nativeType = toWriteMapping(session, column.getColumnType()).getDataType(); + } + return quoted(name) + " " + nativeType; + }).collect(joining(", ")); + String sql = "CREATE TABLE %s (%s, %s Serial, PRIMARY KEY (%s))".formatted( + quoted(newTableName), declarations, quoted(stagingKey), quoted(stagingKey)); + try { + execute(session, connection, sql); + } + catch (SQLException e) { + throw new TrinoException(JDBC_ERROR, "Failed to create YDB INSERT staging table", e); + } + } + @Override public JdbcMergeTableHandle beginMerge( ConnectorSession session, @@ -752,11 +782,16 @@ public JdbcMergeTableHandle beginMerge( if (retryMode != RetryMode.NO_RETRIES) { throw new TrinoException(NOT_SUPPORTED, "Query and task retries are not supported for direct YDB MERGE"); } + if (!isNonTransactionalMerge(session)) { + throw new TrinoException(NOT_SUPPORTED, + "YDB row-level writes require non_transactional_merge=true; completed batches can remain after a failure"); + } List primaryKeys = getPrimaryKeys(session, handle.getRequiredNamedRelation().getRemoteTableName()); if (primaryKeys.isEmpty()) { throw new TrinoException(NOT_SUPPORTED, "The connector cannot perform MERGE on a table without a primary key"); } + updateColumnHandles.values().forEach(columns -> verifyNoPrimaryKeyUpdate(primaryKeys, columns)); SchemaTableName schemaTableName = handle.getRequiredNamedRelation().getSchemaTableName(); RemoteTableName remoteTableName = handle.getRequiredNamedRelation().getRemoteTableName(); @@ -791,12 +826,12 @@ public void finishMerge( JdbcMergeTableHandle tableHandle, Set pageSinkIds ) { - // JdbcMergeSink finishes its operation-specific sinks before reporting success. + // YdbMergeSink has already committed its bounded, non-transactional batches. } @Override public OptionalInt getMaxWriteParallelism(ConnectorSession session) { - // Direct-to-target merge sinks cannot be committed atomically across writer tasks. + // This limits ordinary writes; Trino 483 does not enforce this bound for MERGE. return OptionalInt.of(1); } @@ -819,6 +854,9 @@ public OptionalLong delete(ConnectorSession session, JdbcTableHandle handle) { @Override public OptionalLong update(ConnectorSession session, JdbcTableHandle handle) { + verifyNoPrimaryKeyUpdate( + getPrimaryKeys(session, handle.getRequiredNamedRelation().getRemoteTableName()), + handle.getUpdateAssignments().stream().map(assignment -> (ColumnHandle) assignment.column()).toList()); try (Connection connection = connectionFactory.openConnection(session)) { PreparedQuery preparedQuery = queryBuilder.prepareUpdateQuery( this, @@ -835,6 +873,13 @@ public OptionalLong update(ConnectorSession session, JdbcTableHandle handle) { } } + private static void verifyNoPrimaryKeyUpdate(List primaryKeys, Collection updatedColumns) { + if (updatedColumns.stream().anyMatch(primaryKeys::contains)) { + throw new TrinoException(NOT_SUPPORTED, + "YDB does not support updating primary key columns: https://ydb.tech/docs/en/yql/reference/syntax/update"); + } + } + private long executeReturningDml( ConnectorSession session, Connection connection, diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java index bb6e18fe..932d6565 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java @@ -15,6 +15,7 @@ import io.trino.plugin.jdbc.QueryBuilder; import io.trino.plugin.jdbc.credential.CredentialProvider; import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.connector.ConnectorPageSinkProvider; import tech.ydb.jdbc.YdbDriver; import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder; @@ -35,6 +36,10 @@ public void configure(Binder binder) { .in(Scopes.SINGLETON); bindTablePropertiesProvider(binder, YdbTableProperties.class); + newOptionalBinder(binder, ConnectorPageSinkProvider.class) + .setBinding() + .to(YdbPageSinkProvider.class) + .in(Scopes.SINGLETON); binder.bind(YdbConnector.class).in(Scopes.SINGLETON); } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnector.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnector.java index 851deed6..9074a101 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnector.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnector.java @@ -6,7 +6,11 @@ import io.trino.plugin.jdbc.JdbcConnector; import io.trino.plugin.jdbc.JdbcTransactionManager; import io.trino.plugin.jdbc.TablePropertiesProvider; -import io.trino.spi.connector.*; +import io.trino.spi.connector.ConnectorAccessControl; +import io.trino.spi.connector.ConnectorCapabilities; +import io.trino.spi.connector.ConnectorPageSinkProvider; +import io.trino.spi.connector.ConnectorPageSourceProvider; +import io.trino.spi.connector.ConnectorSplitManager; import io.trino.spi.function.table.ConnectorTableFunction; import io.trino.spi.procedure.Procedure; @@ -16,7 +20,6 @@ import static com.google.common.collect.Sets.immutableEnumSet; import static io.trino.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT; -@SuppressWarnings("all") public class YdbConnector extends JdbcConnector { @Inject public YdbConnector( diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnectorFactory.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnectorFactory.java index b7a68ac5..7aac37a2 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnectorFactory.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbConnectorFactory.java @@ -12,6 +12,8 @@ import java.util.Map; import java.util.function.Supplier; +import static io.trino.plugin.base.Versions.checkStrictSpiVersionMatch; + public record YdbConnectorFactory(String name, Supplier module) implements ConnectorFactory { @Override @@ -21,6 +23,7 @@ public String getName() { @Override public Connector create(String catalogName, Map requiredConfig, ConnectorContext context) { + checkStrictSpiVersionMatch(context, this); Bootstrap app = new Bootstrap( "io.trino.bootstrap.catalog." + catalogName, new ConnectorContextModule(catalogName, context), diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbMergeSink.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbMergeSink.java new file mode 100644 index 00000000..e11706f7 --- /dev/null +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbMergeSink.java @@ -0,0 +1,236 @@ +package tech.ydb.trino; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableSet; +import io.airlift.slice.Slice; +import io.airlift.slice.Slices; +import io.trino.plugin.jdbc.BooleanWriteFunction; +import io.trino.plugin.jdbc.DoubleWriteFunction; +import io.trino.plugin.jdbc.JdbcClient; +import io.trino.plugin.jdbc.JdbcColumnHandle; +import io.trino.plugin.jdbc.JdbcMergeTableHandle; +import io.trino.plugin.jdbc.LongWriteFunction; +import io.trino.plugin.jdbc.ObjectWriteFunction; +import io.trino.plugin.jdbc.SliceWriteFunction; +import io.trino.plugin.jdbc.WriteFunction; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.Page; +import io.trino.spi.TrinoException; +import io.trino.spi.block.Block; +import io.trino.spi.block.RowBlock; +import io.trino.spi.connector.ConnectorMergeSink; +import io.trino.spi.connector.ConnectorPageSinkId; +import io.trino.spi.connector.ConnectorSession; +import io.trino.spi.type.Type; + +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.Collection; +import java.util.HashMap; +import java.util.List; +import java.util.Map; +import java.util.Set; +import java.util.concurrent.CompletableFuture; +import java.util.stream.IntStream; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkState; +import static io.trino.plugin.base.util.Closables.closeAllSuppress; +import static io.trino.plugin.jdbc.JdbcErrorCode.JDBC_ERROR; +import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; +import static io.trino.spi.type.IntegerType.INTEGER; +import static io.trino.spi.type.TinyintType.TINYINT; +import static java.util.concurrent.CompletableFuture.completedFuture; +import static java.util.stream.Collectors.joining; +import static java.util.stream.Collectors.toMap; + +/** + * Implements explicitly non-transactional MERGE with bounded, owned batch transactions. + * No connection or statement is retained between storeMergedRows calls. This is also + * important on Trino 483, where MergeWriterOperator.close does not invoke sink.abort. + */ +final class YdbMergeSink implements ConnectorMergeSink { + private final ConnectorSession session; + private final JdbcMergeTableHandle handle; + private final JdbcClient client; + private final ConnectorPageSinkId pageSinkId; + private final RemoteQueryModifier modifier; + private final int batchSize; + private final List columns; + private final List primaryKeys; + private final Map> updateChannels; + private boolean finished; + + YdbMergeSink(ConnectorSession session, JdbcMergeTableHandle handle, JdbcClient client, + ConnectorPageSinkId pageSinkId, RemoteQueryModifier modifier, int batchSize) { + this.session = session; + this.handle = handle; + this.client = client; + this.pageSinkId = pageSinkId; + this.modifier = modifier; + this.batchSize = batchSize; + columns = handle.getDataColumns(); + primaryKeys = handle.getPrimaryKeys(); + updateChannels = handle.getUpdateCaseColumns().entrySet().stream().collect(toMap( + Map.Entry::getKey, + entry -> IntStream.range(0, columns.size()) + .filter(index -> entry.getValue().contains(columns.get(index))) + .boxed() + .toList())); + } + + @Override + public void storeMergedRows(Page page) { + checkState(!finished, "MERGE sink is finished"); + checkArgument(page.getChannelCount() == columns.size() + 3, "Invalid MERGE page channel count"); + for (int offset = 0; offset < page.getPositionCount(); offset += batchSize) { + writeBatch(page.getRegion(offset, Math.min(batchSize, page.getPositionCount() - offset))); + } + } + + private void writeBatch(Page page) { + try (Connection connection = client.getConnection(session, handle.getOutputTableHandle())) { + connection.setAutoCommit(false); + Map statements = new HashMap<>(); + try { + List columnWriters = writers(connection, columns); + List keyWriters = writers(connection, primaryKeys); + List rowIdFields = RowBlock.getRowFieldsFromBlock(page.getBlock(columns.size() + 2)); + PreparedStatement pending = null; + for (int position = 0; position < page.getPositionCount(); position++) { + int operation = TINYINT.getByte(page.getBlock(columns.size()), position); + int updateCase = operation == UPDATE_OPERATION_NUMBER + ? INTEGER.getInt(page.getBlock(columns.size() + 1), position) : -1; + ImmutableSet.Builder nullKeys = ImmutableSet.builder(); + if (operation != INSERT_OPERATION_NUMBER) { + for (int index = 0; index < primaryKeys.size(); index++) { + if (rowIdFields.get(index).isNull(position)) { + nullKeys.add(index); + } + } + } + StatementKey key = new StatementKey(operation, updateCase, nullKeys.build()); + PreparedStatement statement = statements.get(key); + if (statement == null) { + statement = connection.prepareStatement(modifier.apply(session, sql(key, columnWriters, keyWriters))); + statements.put(key, statement); + } + if (pending != null && pending != statement) { + pending.executeBatch(); + pending.clearBatch(); + } + pending = statement; + int parameter = 1; + switch (operation) { + case INSERT_OPERATION_NUMBER -> { + for (int channel = 0; channel < columns.size(); channel++) { + bind(statement, parameter++, columns.get(channel).getColumnType(), columnWriters.get(channel), page.getBlock(channel), position); + } + } + case UPDATE_OPERATION_NUMBER -> { + for (int channel : updateChannels.get(updateCase)) { + bind(statement, parameter++, columns.get(channel).getColumnType(), columnWriters.get(channel), page.getBlock(channel), position); + } + } + case DELETE_OPERATION_NUMBER -> {} + default -> throw new IllegalArgumentException("Unknown MERGE operation: " + operation); + } + if (operation != INSERT_OPERATION_NUMBER) { + for (int index = 0; index < primaryKeys.size(); index++) { + if (!key.nullKeys().contains(index)) { + bind(statement, parameter++, primaryKeys.get(index).getColumnType(), keyWriters.get(index), rowIdFields.get(index), position); + } + } + } + statement.addBatch(); + } + if (pending != null) { + pending.executeBatch(); + } + connection.commit(); + } + catch (SQLException | RuntimeException e) { + try { + connection.rollback(); + } + catch (SQLException rollbackFailure) { + e.addSuppressed(rollbackFailure); + } + closeAllSuppress(e, statements.values().toArray(PreparedStatement[]::new)); + throw e; + } + SQLException closeFailure = new SQLException("Failed to close YDB MERGE statements"); + closeAllSuppress(closeFailure, statements.values().toArray(PreparedStatement[]::new)); + if (closeFailure.getSuppressed().length > 0) { + throw closeFailure; + } + } + catch (SQLException e) { + throw new TrinoException(JDBC_ERROR, "YDB MERGE batch failed", e); + } + } + + private String sql(StatementKey key, List columnWriters, List keyWriters) { + if (key.operation() == INSERT_OPERATION_NUMBER) { + return client.buildInsertSql(handle.getOutputTableHandle(), columnWriters); + } + String table = client.quoted(handle.getOutputTableHandle().getRemoteTableName()); + String predicate = IntStream.range(0, primaryKeys.size()) + .mapToObj(index -> client.quoted(primaryKeys.get(index).getColumnName()) + + (key.nullKeys().contains(index) ? " IS NULL" : " = " + keyWriters.get(index).getBindExpression())) + .collect(joining(" AND ")); + if (key.operation() == DELETE_OPERATION_NUMBER) { + return "DELETE FROM " + table + " WHERE " + predicate; + } + checkArgument(key.operation() == UPDATE_OPERATION_NUMBER, "Unknown MERGE operation: %s", key.operation()); + List channels = updateChannels.get(key.updateCase()); + checkArgument(channels != null && !channels.isEmpty(), "Unknown or empty MERGE update case: %s", key.updateCase()); + String assignments = channels.stream() + .map(index -> client.quoted(columns.get(index).getColumnName()) + " = " + columnWriters.get(index).getBindExpression()) + .collect(joining(", ")); + return "UPDATE " + table + " SET " + assignments + " WHERE " + predicate; + } + + private List writers(Connection connection, List handles) { + return handles.stream() + .map(column -> client.toColumnMapping(session, connection, column.getJdbcTypeHandle()) + .orElseThrow(() -> new TrinoException(NOT_SUPPORTED, "Unsupported YDB MERGE type: " + column.getJdbcTypeHandle())) + .getWriteFunction()) + .toList(); + } + + private static void bind(PreparedStatement statement, int index, Type type, WriteFunction writer, Block block, int position) + throws SQLException { + if (block.isNull(position)) { + writer.setNull(statement, index); + } else if (type.getJavaType() == boolean.class) { + ((BooleanWriteFunction) writer).set(statement, index, type.getBoolean(block, position)); + } else if (type.getJavaType() == long.class) { + ((LongWriteFunction) writer).set(statement, index, type.getLong(block, position)); + } else if (type.getJavaType() == double.class) { + ((DoubleWriteFunction) writer).set(statement, index, type.getDouble(block, position)); + } else if (type.getJavaType() == Slice.class) { + ((SliceWriteFunction) writer).set(statement, index, type.getSlice(block, position)); + } else { + ((ObjectWriteFunction) writer).set(statement, index, type.getObject(block, position)); + } + } + + @Override + public CompletableFuture> finish() { + checkState(!finished, "MERGE sink is finished"); + finished = true; + Slice fragment = Slices.allocate(Long.BYTES); + fragment.setLong(0, pageSinkId.getId()); + return completedFuture(ImmutableList.of(fragment)); + } + + @Override + public void abort() { + // Completed batches are already committed in the explicitly non-transactional mode. + finished = true; + } + + private record StatementKey(int operation, int updateCase, Set nullKeys) {} +} diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPageSinkProvider.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPageSinkProvider.java new file mode 100644 index 00000000..e0d79323 --- /dev/null +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbPageSinkProvider.java @@ -0,0 +1,40 @@ +package tech.ydb.trino; + +import com.google.inject.Inject; +import io.trino.plugin.jdbc.JdbcClient; +import io.trino.plugin.jdbc.JdbcMergeTableHandle; +import io.trino.plugin.jdbc.JdbcPageSinkProvider; +import io.trino.plugin.jdbc.QueryBuilder; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.connector.ConnectorMergeSink; +import io.trino.spi.connector.ConnectorMergeTableHandle; +import io.trino.spi.connector.ConnectorPageSinkId; +import io.trino.spi.connector.ConnectorSession; +import io.trino.spi.connector.ConnectorTableCredentials; +import io.trino.spi.connector.ConnectorTransactionHandle; + +import java.util.Optional; + +import static io.trino.plugin.jdbc.JdbcWriteSessionProperties.getWriteBatchSize; + +public class YdbPageSinkProvider extends JdbcPageSinkProvider { + private final JdbcClient client; + private final RemoteQueryModifier modifier; + + @Inject + public YdbPageSinkProvider(JdbcClient client, RemoteQueryModifier modifier, QueryBuilder queryBuilder) { + super(client, modifier, queryBuilder); + this.client = client; + this.modifier = modifier; + } + + @Override + public ConnectorMergeSink createMergeSink( + ConnectorTransactionHandle transaction, + ConnectorSession session, + ConnectorMergeTableHandle handle, + Optional credentials, + ConnectorPageSinkId pageSinkId) { + return new YdbMergeSink(session, (JdbcMergeTableHandle) handle, client, pageSinkId, modifier, getWriteBatchSize(session)); + } +} diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorSmokeTest.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorSmokeTest.java index b77ddfb9..a817474f 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorSmokeTest.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorSmokeTest.java @@ -6,11 +6,13 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.parallel.ResourceLock; import tech.ydb.test.junit5.YdbHelperExtension; +@ResourceLock("YDB_HELPER") public class TestYdbConnectorSmokeTest extends BaseConnectorSmokeTest { @RegisterExtension - static final YdbHelperExtension ydb = new YdbHelperExtension(); + static final YdbHelperExtension ydb = new YdbHelperExtension().failIfUnavailable(); @Override protected QueryRunner createQueryRunner() throws Exception { diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java index 8e9f2927..7dd4dcc7 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbConnectorTest.java @@ -12,6 +12,7 @@ import org.assertj.core.api.Assertions; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.parallel.ResourceLock; import tech.ydb.test.junit5.YdbHelperExtension; import java.util.List; @@ -21,10 +22,11 @@ import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; +@ResourceLock("YDB_HELPER") public class TestYdbConnectorTest extends BaseConnectorTest { @RegisterExtension - static final YdbHelperExtension ydb = new YdbHelperExtension(); + static final YdbHelperExtension ydb = new YdbHelperExtension().failIfUnavailable(); @Override protected QueryRunner createQueryRunner() throws Exception { @@ -45,7 +47,7 @@ public void testDefaultSchema() { } @Test - public void testOptInInt64JoinPushdown() { + public void testJoinPushdown() { Session session = Session.builder(getSession()) .setCatalogSessionProperty(getSession().getCatalog().orElseThrow(), "join_pushdown_enabled", "true") .build(); diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java index 6b14a3da..6a1433f8 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java @@ -1,21 +1,71 @@ package tech.ydb.trino; +import com.google.inject.AbstractModule; +import com.google.inject.Provides; +import com.google.inject.Singleton; +import com.google.inject.util.Modules; +import io.trino.Session; +import io.trino.plugin.base.mapping.IdentifierMapping; +import io.trino.plugin.jdbc.BaseJdbcConfig; +import io.trino.plugin.jdbc.ConnectionFactory; +import io.trino.plugin.jdbc.ForBaseJdbc; +import io.trino.plugin.jdbc.JdbcClient; +import io.trino.plugin.jdbc.JdbcOutputTableHandle; +import io.trino.plugin.jdbc.QueryBuilder; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.connector.ConnectorSession; import io.trino.testing.AbstractTestQueryFramework; import io.trino.testing.QueryRunner; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.parallel.ResourceLock; import tech.ydb.test.junit5.YdbHelperExtension; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; + +import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED; +import static java.util.concurrent.TimeUnit.SECONDS; import static org.assertj.core.api.Assertions.assertThat; +@ResourceLock("YDB_HELPER") public class TestYdbCreateTable extends AbstractTestQueryFramework { @RegisterExtension - static final YdbHelperExtension ydb = new YdbHelperExtension(); + static final YdbHelperExtension ydb = new YdbHelperExtension().failIfUnavailable(); + private final ConcurrentMap> rollbacks = new ConcurrentHashMap<>(); @Override protected QueryRunner createQueryRunner() throws Exception { return YdbQueryRunner.builder(ydb) - .useProductionClient() + .addConnectorProperty("insert.non-transactional-insert.enabled", "false") + .addConnectorProperty("merge.non-transactional-merge.enabled", "false") + .setClientModule(Modules.override(new YdbClientModule()).with(new AbstractModule() { + @Provides + @Singleton + @ForBaseJdbc + public JdbcClient client(BaseJdbcConfig config, ConnectionFactory connectionFactory, + QueryBuilder queryBuilder, IdentifierMapping identifierMapping, RemoteQueryModifier modifier) { + return new YdbClient(config, connectionFactory, queryBuilder, identifierMapping, modifier) { + @Override + public void rollbackTemporaryTableCreation(ConnectorSession session, JdbcOutputTableHandle handle) { + CompletableFuture completion = rollbacks.get(handle.getRemoteTableName().getTableName()); + try { + super.rollbackTemporaryTableCreation(session, handle); + if (completion != null) { + completion.complete(null); + } + } + catch (RuntimeException e) { + if (completion != null) { + completion.completeExceptionally(e); + } + throw e; + } + } + }; + } + })) .build(); } @@ -36,7 +86,7 @@ public void testCreateTableWithPrimaryKey() { } @Test - public void testCreateTableAsSelectWithPrimaryKey() { + public void testCreateTableAsSelectWithPrimaryKey() throws Exception { String tableName = "production_ctas_with_pk"; String duplicateTableName = "production_ctas_duplicate_pk"; try { @@ -45,15 +95,61 @@ public void testCreateTableAsSelectWithPrimaryKey() { assertQuery("SELECT * FROM " + tableName, "VALUES (CAST(1 AS BIGINT), 'a'), (CAST(2 AS BIGINT), 'b')"); var tablesBefore = computeActual("SHOW TABLES").getOnlyColumnAsSet(); + CompletableFuture rollback = new CompletableFuture<>(); + rollbacks.put(duplicateTableName, rollback); assertThat(query("CREATE TABLE " + duplicateTableName + " (id, payload) " + "WITH (primary_key = ARRAY['id']) AS VALUES (BIGINT '1', 'a'), (BIGINT '1', 'b')")) .failure(); + rollback.get(30, SECONDS); assertThat(computeActual("SHOW TABLES").getOnlyColumnAsSet()).isEqualTo(tablesBefore); assertThat(getQueryRunner().tableExists(getSession(), duplicateTableName)).isFalse(); } finally { assertUpdate("DROP TABLE IF EXISTS " + tableName); assertUpdate("DROP TABLE IF EXISTS " + duplicateTableName); + rollbacks.remove(duplicateTableName); + } + } + + @Test + public void testTransactionalInsertAndPrimaryKeyUpdateRejection() { + String table = "production_transactional_insert"; + try { + assertUpdate("CREATE TABLE " + table + " (payload varchar, id bigint) WITH (primary_key = ARRAY['id'])"); + assertUpdate("INSERT INTO " + table + " VALUES ('a', 1), ('b', 2)", 2); + var tablesBefore = computeActual("SHOW TABLES").getOnlyColumnAsSet(); + assertThat(query("INSERT INTO " + table + " VALUES ('c', 3), ('duplicate', 1)")).failure(); + assertQuery("SELECT * FROM " + table, "VALUES ('a', BIGINT '1'), ('b', BIGINT '2')"); + assertThat(computeActual("SHOW TABLES").getOnlyColumnAsSet()).isEqualTo(tablesBefore); + assertThat(query("UPDATE " + table + " SET id = 3 WHERE id = 1")).failure().hasErrorCode(NOT_SUPPORTED); + assertUpdate("UPDATE " + table + " SET payload = 'changed' WHERE id = 2", 1); + assertUpdate("DELETE FROM " + table + " WHERE id = 1", 1); + assertQuery("SELECT * FROM " + table, "VALUES ('changed', BIGINT '2')"); + } + finally { + assertUpdate("DROP TABLE IF EXISTS " + table); + } + } + + @Test + public void testExplicitNonTransactionalMergeWithCompositeNullableKey() { + String table = "production_composite_merge"; + String merge = "MERGE INTO " + table + " t USING (VALUES (7, 10, 20), (8, NULL, 30), (9, 11, 40)) s(id, tenant, payload) " + + "ON t.id = s.id AND t.tenant IS NOT DISTINCT FROM s.tenant " + + "WHEN MATCHED THEN UPDATE SET payload = s.payload " + + "WHEN NOT MATCHED THEN INSERT (payload, tenant, id) VALUES (s.payload, s.tenant, s.id)"; + Session mergeSession = Session.builder(getSession()) + .setCatalogSessionProperty("local", "non_transactional_merge", "true").build(); + try { + assertUpdate("CREATE TABLE " + table + " (payload bigint, tenant bigint, id bigint) WITH (primary_key = ARRAY['id', 'tenant'])"); + assertUpdate("INSERT INTO " + table + " VALUES (1, 10, 7), (2, NULL, 8)", 2); + assertThat(query(merge)).failure().hasErrorCode(NOT_SUPPORTED); + assertUpdate(mergeSession, merge, 3); + assertQuery("SELECT * FROM " + table, "VALUES (BIGINT '20', BIGINT '10', BIGINT '7'), (30, NULL, 8), (40, 11, 9)"); + assertUpdate(mergeSession, "DELETE FROM " + table + " WHERE payload + 1 > 0", 3); + } + finally { + assertUpdate("DROP TABLE IF EXISTS " + table); } } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java index 840ccac5..a740b3f2 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java @@ -61,6 +61,15 @@ public void testStringPositionParameterOrder() { .containsExactly(utf8Slice("hello"), utf8Slice("hello")); } + @Test + public void testUnicodeTrimIsNotPushedDown() { + JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(VARCHAR).orElseThrow(), VARCHAR); + JdbcTableHandle table = new JdbcTableHandle(new SchemaTableName("default", "test"), + new RemoteTableName(Optional.empty(), Optional.empty(), "test"), Optional.empty()); + assertThat(client.convertProjection(SESSION, table, + new Call(VARCHAR, new FunctionName("trim"), List.of(new Variable("v", VARCHAR))), Map.of("v", column))).isEmpty(); + } + @Test public void testOverflowingArithmeticIsNotPushedDown() { JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(BIGINT).orElseThrow(), BIGINT); diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java new file mode 100644 index 00000000..2efa0df3 --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java @@ -0,0 +1,164 @@ +package tech.ydb.trino; + +import io.trino.Session; +import io.trino.testing.AbstractTestQueryFramework; +import io.trino.testing.QueryRunner; +import io.trino.testing.sql.JdbcSqlExecutor; +import io.trino.testing.sql.TestTable; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.api.parallel.ResourceLock; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.Arguments; +import org.junit.jupiter.params.provider.MethodSource; +import tech.ydb.test.junit5.YdbHelperExtension; + +import java.util.List; +import java.util.stream.IntStream; +import java.util.stream.Stream; + +import static io.trino.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; +import static java.util.stream.Collectors.joining; +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.jupiter.params.provider.Arguments.arguments; + +@ResourceLock("YDB_HELPER") +public class TestYdbJoinPushdown extends AbstractTestQueryFramework { + @RegisterExtension + static final YdbHelperExtension ydb = new YdbHelperExtension().failIfUnavailable(); + + @Override + protected QueryRunner createQueryRunner() throws Exception { + return YdbQueryRunner.builder(ydb).useProductionClient().build(); + } + + @ParameterizedTest + @MethodSource("scalarKeys") + public void testScalarJoinPushdown(String type, List keys) { + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + try (TestTable left = nativeTable(executor, "join_scalar_left_", "(id Int64, k " + type + ", PRIMARY KEY (id))", "id, k", rows(keys, 0)); + TestTable right = nativeTable(executor, "join_scalar_right_", "(id Int64, k " + type + ", PRIMARY KEY (id))", "id, k", rows(keys, 100))) { + for (String joinType : List.of("INNER", "LEFT", "RIGHT", "FULL")) { + // QueryAssert compares with the same query with connector pushdown disabled. + assertThat(query(joinSession(), "SELECT l.id, r.id FROM " + left.getName() + " l " + + joinType + " JOIN " + right.getName() + " r ON l.k = r.k")).isFullyPushedDown(); + } + assertThat(query(joinSession(), "SELECT l.id, r.id FROM " + left.getName() + " l JOIN " + + right.getName() + " r ON l.k IS NOT DISTINCT FROM r.k")).joinIsNotFullyPushedDown(); + } + } + + public static Stream scalarKeys() { + return Stream.of( + arguments("Bool", List.of("true", "false", "NULL")), + arguments("Int8", List.of("CAST(-128 AS Int8)", "CAST(127 AS Int8)", "NULL")), + arguments("Int16", List.of("CAST(-32768 AS Int16)", "CAST(32767 AS Int16)", "NULL")), + arguments("Int32", List.of("CAST(-2147483648 AS Int32)", "CAST(2147483647 AS Int32)", "NULL")), + arguments("Int64", List.of("-9223372036854775808l", "9223372036854775807l", "NULL")), + arguments("Uint8", List.of("CAST(0 AS Uint8)", "CAST(255 AS Uint8)", "NULL")), + arguments("Uint16", List.of("CAST(0 AS Uint16)", "CAST(65535 AS Uint16)", "NULL")), + arguments("Uint32", List.of("CAST(0 AS Uint32)", "CAST(4294967295ul AS Uint32)", "NULL")), + arguments("Uint64", List.of("0ul", "9223372036854775808ul", "18446744073709551615ul", "NULL")), + arguments("Float", List.of("Float('0')", "Float('-0')", "Float('nan')", "Float('inf')", "Float('-inf')", "NULL")), + arguments("Double", List.of("Double('0')", "Double('-0')", "Double('nan')", "Double('inf')", "Double('-inf')", "NULL")), + arguments("Utf8", List.of("'A'u", "'a'u", "'a 'u", "'\u00e9'u", "'e\u0301'u", "NULL")), + arguments("String", List.of("String('')", "String('\\x00\\x80\\xff')", "NULL")), + arguments("Date", List.of("Date('1970-01-01')", "Date('2105-12-31')", "NULL")), + arguments("Date32", List.of("Date32('1969-12-31')", "Date32('9999-12-31')", "NULL")), + arguments("Datetime", List.of("Datetime('1970-01-01T00:00:00Z')", "Datetime('2020-02-29T12:34:56Z')", "NULL")), + arguments("Datetime64", List.of("Datetime64('1969-12-31T23:59:59Z')", "Datetime64('2020-02-29T12:34:56Z')", "NULL")), + arguments("Timestamp", List.of("Timestamp('1970-01-01T00:00:00Z')", "Timestamp('2020-02-29T12:34:56.123456Z')", "NULL")), + arguments("Timestamp64", List.of("Timestamp64('1969-12-31T23:59:59.999999Z')", "Timestamp64('2020-02-29T12:34:56.123456Z')", "NULL"))); + } + + @Test + public void testDecimalAndComputedKeys() { + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + List values = List.of("1, 1, Decimal('1.25', 22, 9)", "2, 2, NULL"); + try (TestTable left = nativeTable(executor, "join_fallback_left_", "(id Int64, k Int64, d Decimal(22,9), PRIMARY KEY (id))", "id, k, d", values); + TestTable right = nativeTable(executor, "join_fallback_right_", "(id Int64, k Int64, d Decimal(22,9), PRIMARY KEY (id))", "id, k, d", values)) { + String join = "SELECT l.id, r.id FROM " + left.getName() + " l JOIN " + right.getName() + " r ON "; + assertThat(query(joinSession(), join + "l.d = r.d")).joinIsNotFullyPushedDown(); + assertThat(query(joinSession(), join + "l.k + 1 = r.k")).joinIsNotFullyPushedDown(); + assertThat(query(joinSession(), join + "CAST(l.k AS SMALLINT) = CAST(r.k AS SMALLINT)")).joinIsNotFullyPushedDown(); + assertThat(query(joinSession(), join + "l.k < r.k")).joinIsNotFullyPushedDown(); + } + } + + @Test + public void testMixedNativeIntegerKeys() { + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + try (TestTable left = nativeTable(executor, "join_mixed_left_", "(id Int64, k Int16, PRIMARY KEY (id))", "id, k", + List.of("1, -1", "2, 255", "3, NULL")); + TestTable right = nativeTable(executor, "join_mixed_right_", "(id Int64, k Uint8, PRIMARY KEY (id))", "id, k", + List.of("10, 255", "11, 0", "12, NULL"))) { + for (String joinType : List.of("INNER", "LEFT", "RIGHT", "FULL")) { + assertThat(query(joinSession(), "SELECT l.id, r.id FROM " + left.getName() + " l " + + joinType + " JOIN " + right.getName() + " r ON l.k = r.k")).isFullyPushedDown(); + } + } + } + + @Test + public void testUnicodeProjectionAndNulls() { + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + try (TestTable table = nativeTable(executor, "unicode_position_", "(id Int64, s Utf8, sub Utf8, PRIMARY KEY (id))", "id, s, sub", + List.of("1, '\u00e9x'u, 'x'u", "2, NULL, 'x'u", "3, 'x'u, NULL", "4, 'x'u, 'missing'u"))) { + assertThat(query("SELECT id, strpos(s, sub) FROM " + table.getName())) + .matches("VALUES (BIGINT '1', BIGINT '2'), (2, NULL), (3, NULL), (4, 0)") + .isFullyPushedDown(); + } + } + + @Test + public void testFloatingPointTopN() { + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + try (TestTable table = nativeTable(executor, "float_topn_", "(id Int64, k Double, PRIMARY KEY (id))", "id, k", + rows(List.of("Double('nan')", "Double('-0')", "Double('0')", "Double('-inf')", "Double('inf')", "NULL"), 0))) { + for (String order : List.of("ASC NULLS FIRST", "ASC NULLS LAST", "DESC NULLS FIRST", "DESC NULLS LAST")) { + assertThat(query("SELECT id, k FROM " + table.getName() + " ORDER BY k " + order + ", id LIMIT 5")) + .ordered() + .isFullyPushedDown(); + } + } + } + + @Test + public void testOverflowIsEvaluatedByTrino() { + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + try (TestTable left = nativeTable(executor, "join_overflow_left_", "(id Int64, k Int64, PRIMARY KEY (id))", "id, k", + List.of("1, 9223372036854775807l", "2, -9223372036854775808l")); + TestTable right = nativeTable(executor, "join_overflow_right_", "(id Int64, k Int64, PRIMARY KEY (id))", "id, k", List.of("1, 0"))) { + String join = "SELECT l.id FROM " + left.getName() + " l JOIN " + right.getName() + " r ON "; + assertThat(query(joinSession(), join + "l.k + 1 = r.k")).failure().hasErrorCode(NUMERIC_VALUE_OUT_OF_RANGE); + assertThat(query(joinSession(), join + "l.k / -1 = r.k")).failure().hasErrorCode(NUMERIC_VALUE_OUT_OF_RANGE); + assertThat(query(joinSession(), join + "CAST(l.k AS SMALLINT) = r.k")).failure().hasErrorCode(NUMERIC_VALUE_OUT_OF_RANGE); + } + } + + private Session joinSession() { + return Session.builder(getSession()).setCatalogSessionProperty("local", "join_pushdown_enabled", "true").build(); + } + + private static TestTable nativeTable(JdbcSqlExecutor executor, String prefix, String definition, String columns, List rows) { + TestTable table = new TestTable(executor, prefix, definition); + try { + executor.execute("INSERT INTO " + table.getName() + " (" + columns + ") VALUES " + + rows.stream().map(row -> "(" + row + ")").collect(joining(", "))); + return table; + } + catch (RuntimeException e) { + try { + table.close(); + } + catch (RuntimeException closeFailure) { + e.addSuppressed(closeFailure); + } + throw e; + } + } + + private static List rows(List keys, int offset) { + return IntStream.range(0, keys.size()).mapToObj(index -> (offset + index + 1) + ", " + keys.get(index)).toList(); + } +} diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbMergeSink.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbMergeSink.java new file mode 100644 index 00000000..d2138270 --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbMergeSink.java @@ -0,0 +1,217 @@ +package tech.ydb.trino; + +import io.trino.plugin.base.mapping.DefaultIdentifierMapping; +import io.trino.plugin.jdbc.BaseJdbcConfig; +import io.trino.plugin.jdbc.JdbcColumnHandle; +import io.trino.plugin.jdbc.JdbcMergeTableHandle; +import io.trino.plugin.jdbc.JdbcOutputTableHandle; +import io.trino.plugin.jdbc.JdbcTableHandle; +import io.trino.plugin.jdbc.RemoteTableName; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.Page; +import io.trino.spi.block.Block; +import io.trino.spi.block.RowBlock; +import io.trino.spi.connector.SchemaTableName; +import io.trino.spi.type.Type; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.SQLException; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import java.util.TreeMap; + +import static io.trino.spi.connector.ConnectorMergeSink.DELETE_OPERATION_NUMBER; +import static io.trino.spi.connector.ConnectorMergeSink.INSERT_OPERATION_NUMBER; +import static io.trino.spi.connector.ConnectorMergeSink.UPDATE_OPERATION_NUMBER; +import static io.trino.spi.type.BigintType.BIGINT; +import static io.trino.spi.type.IntegerType.INTEGER; +import static io.trino.spi.type.TinyintType.TINYINT; +import static io.trino.testing.TestingConnectorSession.SESSION; +import static org.assertj.core.api.Assertions.assertThat; +import static org.assertj.core.api.Assertions.assertThatThrownBy; + +public class TestYdbMergeSink { + @Test + public void testOperationOrderCompositeNullKeysAndResourceClosure() { + RecordingJdbc jdbc = new RecordingJdbc(0); + YdbMergeSink sink = sink(jdbc, 1000); + assertThat(jdbc.connections).isZero(); + sink.storeMergedRows(page()); + assertThat(jdbc.executions).hasSize(4); + assertThat(jdbc.executions.get(0).parameters()).containsExactly(90L, 10L, 7L); + assertThat(jdbc.executions.get(1).sql()).contains("`id` = ? AND `tenant` IS NULL"); + assertThat(jdbc.executions.get(1).parameters()).containsExactly(91L, 7L); + assertThat(jdbc.executions.get(2).parameters()).containsExactly(92L, 11L, 8L); + assertThat(jdbc.executions.get(3).parameters()).containsExactly(9L, 12L); + assertThat(jdbc.commits).isEqualTo(1); + assertThat(jdbc.rollbacks).isZero(); + assertThat(jdbc.connections).isEqualTo(1); + assertThat(jdbc.closedConnections).isEqualTo(1); + assertThat(jdbc.closedStatements).isEqualTo(3); + assertThat(sink.finish().join()).hasSize(1); + sink.abort(); + assertThat(jdbc.commits).isEqualTo(1); + assertThat(jdbc.connections).isEqualTo(1); + } + + @Test + public void testFailureRollsBackCurrentBatchWithoutReplay() { + RecordingJdbc jdbc = new RecordingJdbc(2); + YdbMergeSink sink = sink(jdbc, 1000); + assertThatThrownBy(() -> sink.storeMergedRows(page())).hasCause(jdbc.failure); + assertThat(jdbc.executions).hasSize(2); + assertThat(jdbc.commits).isZero(); + assertThat(jdbc.rollbacks).isEqualTo(1); + assertThat(jdbc.connections).isEqualTo(1); + assertThat(jdbc.closedConnections).isEqualTo(1); + assertThat(jdbc.closedStatements).isEqualTo(2); + sink.abort(); + assertThat(jdbc.rollbacks).isEqualTo(1); + } + + @Test + public void testBoundedNonTransactionalBatches() { + RecordingJdbc jdbc = new RecordingJdbc(4); + YdbMergeSink sink = sink(jdbc, 2); + assertThatThrownBy(() -> sink.storeMergedRows(page())).hasCause(jdbc.failure); + assertThat(jdbc.executions).hasSize(4); + assertThat(jdbc.commits).isEqualTo(1); + assertThat(jdbc.rollbacks).isEqualTo(1); + assertThat(jdbc.connections).isEqualTo(2); + assertThat(jdbc.closedConnections).isEqualTo(2); + assertThat(jdbc.closedStatements).isEqualTo(4); + } + + @Test + public void testAbortBeforeInputHasNoConnectionToLeak() { + RecordingJdbc jdbc = new RecordingJdbc(0); + YdbMergeSink sink = sink(jdbc, 1000); + sink.abort(); + assertThat(jdbc.connections).isZero(); + assertThatThrownBy(() -> sink.storeMergedRows(page())).isInstanceOf(IllegalStateException.class); + } + + private static YdbMergeSink sink(RecordingJdbc jdbc, int batchSize) { + YdbClient client = new YdbClient(new BaseJdbcConfig(), _ -> jdbc.connection(), + new YdbQueryBuilder(RemoteQueryModifier.NONE), new DefaultIdentifierMapping(), RemoteQueryModifier.NONE); + List columns = List.of("payload", "tenant", "id").stream() + .map(name -> new JdbcColumnHandle(name, YdbTypeUtils.toTypeHandle(BIGINT).orElseThrow(), BIGINT)) + .toList(); + RemoteTableName remote = new RemoteTableName(Optional.empty(), Optional.empty(), "test"); + JdbcOutputTableHandle output = new JdbcOutputTableHandle(remote, List.of("payload", "tenant", "id"), + List.of(BIGINT, BIGINT, BIGINT), Optional.of(columns.stream().map(JdbcColumnHandle::getJdbcTypeHandle).toList()), + Optional.empty(), Optional.empty()); + JdbcMergeTableHandle handle = new JdbcMergeTableHandle( + new JdbcTableHandle(new SchemaTableName("default", "test"), remote, Optional.empty()), output, Map.of(), Optional.empty(), + List.of(columns.get(2), columns.get(1)), columns, Map.of(0, List.of(columns.getFirst()))); + return new YdbMergeSink(SESSION, handle, client, () -> 123, RemoteQueryModifier.NONE, batchSize); + } + + private static Page page() { + return new Page( + numbers(BIGINT, 90L, 91L, 92L, 93L), + numbers(BIGINT, 10L, null, 11L, 12L), + numbers(BIGINT, 7L, 7L, 8L, 9L), + numbers(TINYINT, (long) INSERT_OPERATION_NUMBER, (long) UPDATE_OPERATION_NUMBER, + (long) INSERT_OPERATION_NUMBER, (long) DELETE_OPERATION_NUMBER), + numbers(INTEGER, 0L, 0L, 0L, 0L), + RowBlock.fromFieldBlocks(4, new Block[]{numbers(BIGINT, 7L, 7L, 8L, 9L), numbers(BIGINT, 10L, null, 11L, 12L)})); + } + + private static Block numbers(Type type, Long... values) { + var builder = type.createBlockBuilder(null, values.length); + for (Long value : values) { + if (value == null) { + builder.appendNull(); + } else { + type.writeLong(builder, value); + } + } + return builder.build(); + } + + private record Execution(String sql, List parameters) {} + + private static final class RecordingJdbc { + private final int failOnExecution; + private final SQLException failure = new SQLException("Injected statement failure"); + private final List executions = new ArrayList<>(); + private int commits; + private int rollbacks; + private int connections; + private int closedConnections; + private int closedStatements; + + private RecordingJdbc(int failOnExecution) { + this.failOnExecution = failOnExecution; + } + + private Connection connection() { + connections++; + return (Connection) Proxy.newProxyInstance(Connection.class.getClassLoader(), new Class[]{Connection.class}, + (_, method, args) -> switch (method.getName()) { + case "setAutoCommit" -> { + assertThat(args[0]).isEqualTo(false); + yield null; + } + case "prepareStatement" -> statement((String) args[0]); + case "commit" -> { + commits++; + yield null; + } + case "rollback" -> { + rollbacks++; + yield null; + } + case "close" -> { + closedConnections++; + yield null; + } + default -> throw new UnsupportedOperationException(method.getName()); + }); + } + + private PreparedStatement statement(String sql) { + Map parameters = new TreeMap<>(); + List> batch = new ArrayList<>(); + return (PreparedStatement) Proxy.newProxyInstance(PreparedStatement.class.getClassLoader(), new Class[]{PreparedStatement.class}, + (_, method, args) -> switch (method.getName()) { + case "setLong", "setObject" -> { + parameters.put((int) args[0], args[1]); + yield null; + } + case "setNull" -> { + parameters.put((int) args[0], null); + yield null; + } + case "addBatch" -> { + batch.add(new ArrayList<>(parameters.values())); + yield null; + } + case "clearBatch" -> { + batch.clear(); + yield null; + } + case "executeBatch" -> { + for (List row : batch) { + executions.add(new Execution(sql, row)); + if (executions.size() == failOnExecution) { + throw failure; + } + } + yield new int[batch.size()]; + } + case "close" -> { + closedStatements++; + yield null; + } + default -> throw new UnsupportedOperationException(method.getName()); + }); + } + } +} diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbWriteMetadata.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbWriteMetadata.java new file mode 100644 index 00000000..0332978e --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbWriteMetadata.java @@ -0,0 +1,74 @@ +package tech.ydb.trino; + +import io.trino.plugin.base.mapping.DefaultIdentifierMapping; +import io.trino.plugin.jdbc.BaseJdbcConfig; +import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import org.junit.jupiter.api.Test; + +import java.lang.reflect.Proxy; +import java.sql.Connection; +import java.sql.DatabaseMetaData; +import java.sql.ResultSet; +import java.sql.Statement; +import java.sql.Types; +import java.util.ArrayList; +import java.util.List; +import java.util.Map; +import java.util.concurrent.atomic.AtomicBoolean; +import java.util.function.Supplier; + +import static io.trino.testing.TestingConnectorSession.SESSION; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestYdbWriteMetadata { + @Test + public void testStagingUsesDriverTableIdentity() throws Exception { + Map row = Map.of( + "TABLE_NAME", "source", + "COLUMN_NAME", "payload", + "DATA_TYPE", Types.BIGINT, + "TYPE_NAME", "Int64", + "COLUMN_SIZE", 19, + "DECIMAL_DIGITS", 0, + "NULLABLE", DatabaseMetaData.columnNullable); + AtomicBoolean firstRow = new AtomicBoolean(true); + ResultSet result = (ResultSet) Proxy.newProxyInstance(ResultSet.class.getClassLoader(), new Class[]{ResultSet.class}, + (_, method, args) -> switch (method.getName()) { + case "next" -> firstRow.getAndSet(false); + case "getString", "getInt" -> row.get(args[0]); + case "wasNull" -> false; + case "close" -> null; + default -> throw new UnsupportedOperationException(method.getName()); + }); + DatabaseMetaData metadata = (DatabaseMetaData) Proxy.newProxyInstance(DatabaseMetaData.class.getClassLoader(), new Class[]{DatabaseMetaData.class}, + (_, method, _) -> switch (method.getName()) { + case "getColumns" -> result; + case "getSearchStringEscape" -> "\\"; + default -> throw new UnsupportedOperationException(method.getName()); + }); + List commands = new ArrayList<>(); + Statement statement = (Statement) Proxy.newProxyInstance(Statement.class.getClassLoader(), new Class[]{Statement.class}, + (_, method, args) -> { + if (method.getName().equals("execute")) { + commands.add((String) args[0]); + return false; + } + assertThat(method.getName()).isEqualTo("close"); + return null; + }); + Supplier connection = () -> (Connection) Proxy.newProxyInstance(Connection.class.getClassLoader(), new Class[]{Connection.class}, + (_, method, _) -> switch (method.getName()) { + case "getMetaData" -> metadata; + case "createStatement" -> statement; + case "close" -> null; + default -> throw new UnsupportedOperationException(method.getName()); + }); + YdbClient client = new YdbClient(new BaseJdbcConfig(), _ -> connection.get(), + new YdbQueryBuilder(RemoteQueryModifier.NONE), new DefaultIdentifierMapping(), RemoteQueryModifier.NONE); + try (Connection target = connection.get()) { + client.copyTableSchema(SESSION, target, null, "default", "source", "staging", List.of("payload")); + } + assertThat(commands).containsExactly( + "CREATE TABLE `staging` (`payload` Int64, `_trino_ydb_staging_key` Serial, PRIMARY KEY (`_trino_ydb_staging_key`))"); + } +} diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java index 3d947848..60f0c3d8 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestingYdbJdbcModule.java @@ -14,6 +14,7 @@ import io.trino.plugin.jdbc.QueryBuilder; import io.trino.plugin.jdbc.credential.CredentialProvider; import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.connector.ConnectorPageSinkProvider; import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder; import static io.trino.plugin.jdbc.JdbcModule.bindTablePropertiesProvider; @@ -33,6 +34,10 @@ public void configure(Binder binder) { .in(Scopes.SINGLETON); bindTablePropertiesProvider(binder, YdbTableProperties.class); + newOptionalBinder(binder, ConnectorPageSinkProvider.class) + .setBinding() + .to(YdbPageSinkProvider.class) + .in(Scopes.SINGLETON); binder.bind(YdbConnector.class).in(Scopes.SINGLETON); } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/YdbQueryRunner.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/YdbQueryRunner.java index 4046661b..c274004a 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/YdbQueryRunner.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/YdbQueryRunner.java @@ -2,6 +2,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.inject.Module; import io.trino.Session; import io.trino.testing.DistributedQueryRunner; import io.trino.testing.MaterializedResult; @@ -29,6 +30,7 @@ public static Builder builder(YdbHelperExtension ydb) { return new Builder() // Avoid temporary-table CTAS during INSERT; YDB does not support CREATE TABLE AS SELECT. .addConnectorProperty("insert.non-transactional-insert.enabled", "true") + .addConnectorProperty("merge.non-transactional-merge.enabled", "true") .addConnectorProperty("connection-url", jdbcUrl); } @@ -48,6 +50,7 @@ public static class Builder extends DistributedQueryRunner.Builder { private final Map connectorProperties = new HashMap<>(); private List> initialTables = ImmutableList.of(); private boolean useTestingClient = true; + private Module clientModule; private Builder() { super(testSessionBuilder() @@ -71,6 +74,11 @@ public Builder useProductionClient() { return this; } + public Builder setClientModule(Module clientModule) { + this.clientModule = clientModule; + return this; + } + @Override public DistributedQueryRunner build() throws Exception { DistributedQueryRunner queryRunner = super.build(); @@ -78,7 +86,8 @@ public DistributedQueryRunner build() throws Exception { queryRunner.installPlugin(new io.trino.plugin.tpch.TpchPlugin()); queryRunner.createCatalog("tpch", "tpch"); - queryRunner.installPlugin(new YdbPlugin(useTestingClient ? new TestingYdbJdbcModule() : new YdbClientModule())); + queryRunner.installPlugin(new YdbPlugin(clientModule != null + ? clientModule : useTestingClient ? new TestingYdbJdbcModule() : new YdbClientModule())); queryRunner.createCatalog("local", "ydb", ImmutableMap.copyOf(connectorProperties)); for (TpchTable table : initialTables) { From 2cd8bbede0242ef1a08844e8477f9f5ca02d47e1 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 18:06:41 +0300 Subject: [PATCH 08/13] Fix YDB ordering and concurrent connection lifecycle 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. --- ydb-trino-adapter/README.md | 13 ++++++-- ydb-trino-adapter/ROADMAP.md | 9 ++++-- ydb-trino-adapter/UPSTREAM-READINESS.md | 13 ++++++-- .../main/java/tech/ydb/trino/YdbClient.java | 3 +- .../java/tech/ydb/trino/YdbClientModule.java | 6 ++++ .../tech/ydb/trino/YdbColumnMappings.java | 14 ++++++++- .../tech/ydb/trino/TestYdbColumnMappings.java | 30 +++++++++++++++++++ .../tech/ydb/trino/TestYdbCreateTable.java | 6 ++-- .../tech/ydb/trino/TestYdbJoinMappings.java | 30 +++++++++++++++---- .../java/tech/ydb/trino/TestYdbPlugin.java | 27 +++++++++++++++++ 10 files changed, 135 insertions(+), 16 deletions(-) create mode 100644 ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbPlugin.java diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index 94984cb2..0f902163 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -139,5 +139,14 @@ YDB `Text` отображается в Trino как `varchar`, а `Bytes` — к `Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp`/`Timestamp64` — `Instant` с микросекундами. В построчных UPDATE/DELETE собственный merge sink использует исходный `JdbcTypeHandle` и те же native write mappings, что и обычная запись. -Предикаты для `Date`/`Date32` и legacy temporal типов оставляются в Trino, -чтобы не связывать значения вне диапазона исходного YDB-типа. +Date32-предикаты передаются в YDB только с границами внутри документированного +диапазона. Предикаты с выходящими за него значениями, а также предикаты +для legacy temporal типов остаются в Trino. + +## JDBC-контексты + +Для JDBC 2.4.1 адаптер задаёт `cacheConnectionsInDriver=false`: при одновременном +открытии и закрытии последнего соединения cache драйвера может вернуть уже +закрываемый контекст. Каждый JDBC connection поэтому владеет своим контекстом. +Это не retry и не replay записей. Параметры JDBC URL имеют приоритет над +properties; не включайте `cacheConnectionsInDriver=true` на этой версии драйвера. diff --git a/ydb-trino-adapter/ROADMAP.md b/ydb-trino-adapter/ROADMAP.md index 334314b1..0ff36f75 100644 --- a/ydb-trino-adapter/ROADMAP.md +++ b/ydb-trino-adapter/ROADMAP.md @@ -101,8 +101,13 @@ or verified upstream planner and cancellation changes, not an implicit promise. failure. A future retry design requires staging or an operation ID that proves replay safety. See [YDB SDK error handling](https://ydb.tech/docs/en/reference/ydb-sdk/error_handling). -- The connector uses the YDB JDBC 2.4.1 default - `cacheConnectionsInDriver=true`; an explicit JDBC URL option can override it. +- The connector defaults `cacheConnectionsInDriver=false` for JDBC 2.4.1. + `YdbDriver.connect` obtains a cached context before connection registration, + while the last connection close can remove and shut down that same context. + CI exposed a terminated session-pool executor during a concurrent UPDATE. + Per-connection context ownership avoids this race without replay. URL options + override Properties; explicitly re-enabling this cache is not supported. + See the [pinned driver lifecycle](https://github.com/ydb-platform/ydb-jdbc-driver/blob/v2.4.1/jdbc/src/main/java/tech/ydb/jdbc/YdbDriver.java). - Do not replay buffered INSERT pages after `JdbcPageSink` may already have committed an internal batch. - Add tests for partial sink failures and abort cleanup. diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md index 4910849a..12d6ee7d 100644 --- a/ydb-trino-adapter/UPSTREAM-READINESS.md +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -78,12 +78,12 @@ no replacement upstream PR is created by this work. ## Validation -With Temurin 25.0.2, the following standalone command passed 18 tests, with +With Temurin 25.0.2, the following standalone command passed 20 tests, with zero failures, errors or skips, and built the module: ```bash mvn -B -ntp -f ydb-trino-adapter/pom.xml \ - -Dtest=TestYdbExpressionRewrites,TestYdbColumnMappings,TestYdbJoinMappings,TestYdbMergeSink,TestYdbWriteMetadata \ + -Dtest=TestYdbExpressionRewrites,TestYdbColumnMappings,TestYdbJoinMappings,TestYdbMergeSink,TestYdbWriteMetadata,TestYdbPlugin \ verify ``` @@ -99,5 +99,14 @@ the ordinary module CI: mvn -B -ntp -f ydb-trino-adapter/pom.xml clean test ``` +Canonical PR 270's first real-YDB run (`36731448784`) reported 354 tests, +4 failures, 1 error and 85 skips. The scalar JOIN matrix and smoke suite ran. +It exposed NaN ordering dependent on both sort direction and NULL placement, +overly broad Date32 predicate fallback, a cached JDBC context-close race, and +two H2 reference queries using unsupported typed-literal syntax. +The follow-up checks NaN ordering against the actual Trino type operator, +pushes only safely bounded Date32 predicates, defaults to uncached JDBC +contexts, and corrects the H2 syntax. Its full real-YDB rerun is still required. + The current PR's exact-head CI and independent review, not the historical prototype or earlier PRs, determine whether this branch is ready to merge. diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index 912c65f5..893434c6 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -570,7 +570,8 @@ protected Optional topNFunction() { String direction = sortOrder.isAscending() ? "ASC" : "DESC"; Type type = sortItem.column().getColumnType(); if (type.equals(REAL) || type.equals(DOUBLE)) { - String nanSort = "CASE WHEN %1$s != %1$s THEN 1 ELSE 0 END %2$s".formatted(columnName, direction); + String nanDirection = sortOrder.isNullsFirst() == sortOrder.isAscending() ? "DESC" : "ASC"; + String nanSort = "CASE WHEN %1$s != %1$s THEN 1 ELSE 0 END %2$s".formatted(columnName, nanDirection); String yqlType = type.equals(REAL) ? "Float" : "Double"; return Stream.of(nullSort, nanSort, "NANVL(%s, CAST(0 AS %s)) %s".formatted(columnName, yqlType, direction)); } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java index 932d6565..02696bb0 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java @@ -18,6 +18,8 @@ import io.trino.spi.connector.ConnectorPageSinkProvider; import tech.ydb.jdbc.YdbDriver; +import java.util.Properties; + import static com.google.inject.multibindings.OptionalBinder.newOptionalBinder; import static io.trino.plugin.jdbc.JdbcModule.bindTablePropertiesProvider; @@ -61,10 +63,14 @@ public JdbcClient provideJdbcClient( public static ConnectionFactory createConnectionFactory( BaseJdbcConfig config, CredentialProvider credentialProvider) { + // JDBC 2.4.1 can close a cached context between lookup and connection registration. + Properties properties = new Properties(); + properties.setProperty("cacheConnectionsInDriver", "false"); return DriverConnectionFactory.builder( new YdbDriver(), config.getConnectionUrl(), credentialProvider) + .setConnectionProperties(properties) .build(); } } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java index b774ddfe..4f3c2552 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java @@ -15,6 +15,7 @@ import java.sql.SQLDataException; import java.sql.SQLException; import java.time.Instant; +import java.time.LocalDate; import java.time.LocalDateTime; import java.util.Locale; @@ -40,6 +41,9 @@ final class YdbColumnMappings { static final int YDB_DATE32_SQL_TYPE = SQL_KIND_PRIMITIVE + 25; private static final int YDB_DATETIME64_SQL_TYPE = SQL_KIND_PRIMITIVE + 26; static final int YDB_TIMESTAMP64_SQL_TYPE = SQL_KIND_PRIMITIVE + 27; + // The documented Date32 interval; keep the upper endpoint exclusive. + private static final long MIN_DATE32_DAY = LocalDate.of(-144168, 1, 1).toEpochDay(); + private static final long MAX_DATE32_DAY = LocalDate.of(148107, 1, 1).toEpochDay() - 1; private YdbColumnMappings() {} @@ -54,7 +58,15 @@ static ColumnMapping dateColumnMapping(JdbcTypeHandle typeHandle) { DATE, dateReadFunctionUsingLocalDate(), writeFunction, - DISABLE_PUSHDOWN); + (session, domain) -> { + boolean safeBounds = typeName.equalsIgnoreCase("Date32") && + domain.getValues().getRanges().getOrderedRanges().stream().allMatch(range -> + (range.isLowUnbounded() || ((long) range.getLowBoundedValue() >= MIN_DATE32_DAY && + (long) range.getLowBoundedValue() <= MAX_DATE32_DAY)) && + (range.isHighUnbounded() || ((long) range.getHighBoundedValue() >= MIN_DATE32_DAY && + (long) range.getHighBoundedValue() <= MAX_DATE32_DAY))); + return (safeBounds ? FULL_PUSHDOWN : DISABLE_PUSHDOWN).apply(session, domain); + }); } static ColumnMapping timestampColumnMapping(JdbcTypeHandle typeHandle) { diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java index 2a714c15..e3797342 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java @@ -4,12 +4,18 @@ import io.trino.plugin.jdbc.BaseJdbcConfig; import io.trino.plugin.jdbc.ColumnMapping; import io.trino.plugin.jdbc.JdbcTypeHandle; +import io.trino.plugin.jdbc.JdbcMetadataConfig; +import io.trino.plugin.jdbc.JdbcMetadataSessionProperties; import io.trino.plugin.jdbc.LongReadFunction; import io.trino.plugin.jdbc.LongWriteFunction; import io.trino.plugin.jdbc.ObjectReadFunction; import io.trino.plugin.jdbc.ObjectWriteFunction; import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.predicate.Domain; +import io.trino.spi.predicate.Range; +import io.trino.spi.predicate.ValueSet; import io.trino.spi.type.Int128; +import io.trino.testing.TestingConnectorSession; import org.junit.jupiter.api.Test; import java.lang.reflect.Proxy; @@ -21,6 +27,7 @@ import java.sql.SQLException; import java.sql.Types; import java.time.Instant; +import java.time.LocalDate; import java.time.LocalDateTime; import java.util.ArrayList; import java.util.Arrays; @@ -28,6 +35,7 @@ import java.util.Optional; import static io.trino.spi.type.DecimalType.createDecimalType; +import static io.trino.spi.type.DateType.DATE; import static io.trino.spi.type.SmallintType.SMALLINT; import static io.trino.testing.TestingConnectorSession.SESSION; import static org.assertj.core.api.Assertions.assertThat; @@ -116,6 +124,28 @@ public void testUtcTimestampReads() throws Exception { } } + @Test + public void testDate32PredicateBounds() { + var session = TestingConnectorSession.builder().setPropertyMetadata( + new JdbcMetadataSessionProperties(new JdbcMetadataConfig(), Optional.empty()).getSessionProperties()).build(); + JdbcTypeHandle handle = new JdbcTypeHandle(Types.DATE, Optional.of("Date32"), + Optional.empty(), Optional.empty(), Optional.empty(), Optional.empty()); + var controller = YdbColumnMappings.dateColumnMapping(handle).getPredicatePushdownController(); + long day = LocalDate.of(1995, 9, 16).toEpochDay(); + for (Domain domain : List.of(Domain.singleValue(DATE, day), Domain.onlyNull(DATE), + Domain.create(ValueSet.ofRanges(Range.greaterThan(DATE, day)), false))) { + var result = controller.apply(session, domain); + assertThat(result.getPushedDown()).isEqualTo(domain); + assertThat(result.getRemainingFilter().isAll()).isTrue(); + } + for (Domain domain : List.of(Domain.singleValue(DATE, (long) Integer.MIN_VALUE), + Domain.create(ValueSet.ofRanges(Range.lessThan(DATE, (long) Integer.MAX_VALUE)), true))) { + var result = controller.apply(session, domain); + assertThat(result.getPushedDown().isAll()).isTrue(); + assertThat(result.getRemainingFilter()).isEqualTo(domain); + } + } + @Test public void testDatetimeRejectsFractionalSeconds() { JdbcTypeHandle handle = new JdbcTypeHandle(Types.TIMESTAMP, Optional.of("Datetime64"), diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java index 6a1433f8..417dbb2f 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbCreateTable.java @@ -119,12 +119,12 @@ public void testTransactionalInsertAndPrimaryKeyUpdateRejection() { assertUpdate("INSERT INTO " + table + " VALUES ('a', 1), ('b', 2)", 2); var tablesBefore = computeActual("SHOW TABLES").getOnlyColumnAsSet(); assertThat(query("INSERT INTO " + table + " VALUES ('c', 3), ('duplicate', 1)")).failure(); - assertQuery("SELECT * FROM " + table, "VALUES ('a', BIGINT '1'), ('b', BIGINT '2')"); + assertQuery("SELECT * FROM " + table, "VALUES ('a', CAST(1 AS BIGINT)), ('b', CAST(2 AS BIGINT))"); assertThat(computeActual("SHOW TABLES").getOnlyColumnAsSet()).isEqualTo(tablesBefore); assertThat(query("UPDATE " + table + " SET id = 3 WHERE id = 1")).failure().hasErrorCode(NOT_SUPPORTED); assertUpdate("UPDATE " + table + " SET payload = 'changed' WHERE id = 2", 1); assertUpdate("DELETE FROM " + table + " WHERE id = 1", 1); - assertQuery("SELECT * FROM " + table, "VALUES ('changed', BIGINT '2')"); + assertQuery("SELECT * FROM " + table, "VALUES ('changed', CAST(2 AS BIGINT))"); } finally { assertUpdate("DROP TABLE IF EXISTS " + table); @@ -145,7 +145,7 @@ public void testExplicitNonTransactionalMergeWithCompositeNullableKey() { assertUpdate("INSERT INTO " + table + " VALUES (1, 10, 7), (2, NULL, 8)", 2); assertThat(query(merge)).failure().hasErrorCode(NOT_SUPPORTED); assertUpdate(mergeSession, merge, 3); - assertQuery("SELECT * FROM " + table, "VALUES (BIGINT '20', BIGINT '10', BIGINT '7'), (30, NULL, 8), (40, 11, 9)"); + assertQuery("SELECT * FROM " + table, "VALUES (CAST(20 AS BIGINT), CAST(10 AS BIGINT), CAST(7 AS BIGINT)), (30, NULL, 8), (40, 11, 9)"); assertUpdate(mergeSession, "DELETE FROM " + table + " WHERE payload + 1 > 0", 3); } finally { diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java index 35d4e794..43f8f99c 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinMappings.java @@ -7,8 +7,10 @@ import io.trino.plugin.jdbc.JdbcSortItem; import io.trino.plugin.jdbc.JdbcTypeHandle; import io.trino.plugin.jdbc.logging.RemoteQueryModifier; +import io.trino.spi.connector.SortOrder; import io.trino.spi.type.ArrayType; import io.trino.spi.type.Type; +import io.trino.spi.type.TypeOperators; import org.junit.jupiter.api.Test; import java.sql.SQLException; @@ -19,7 +21,9 @@ import static io.trino.spi.connector.JoinCondition.Operator.EQUAL; import static io.trino.spi.connector.JoinCondition.Operator.IDENTICAL; import static io.trino.spi.connector.JoinCondition.Operator.LESS_THAN; -import static io.trino.spi.connector.SortOrder.ASC_NULLS_LAST; +import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.BLOCK_POSITION; +import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL; +import static io.trino.spi.function.InvocationConvention.simpleConvention; import static io.trino.spi.type.BigintType.BIGINT; import static io.trino.spi.type.DecimalType.createDecimalType; import static io.trino.spi.type.DoubleType.DOUBLE; @@ -63,16 +67,32 @@ public void testDecimalAndMixedTypesFallBack() { } @Test - public void testFloatingPointNormalizationAndTypedTopN() { + public void testFloatingPointNormalizationAndTypedTopN() throws Throwable { YdbQueryBuilder queryBuilder = new YdbQueryBuilder(RemoteQueryModifier.NONE); for (Type type : List.of(REAL, DOUBLE)) { String nativeType = type.equals(REAL) ? "Float" : "Double"; JdbcColumnHandle column = new JdbcColumnHandle("key", typeHandle(nativeType), type); String condition = queryBuilder.formatJoinCondition(client, "l", "r", new JdbcJoinCondition(column, EQUAL, column)); assertThat(condition).contains("NANVL(", "CAST(NULL AS " + nativeType + ")", "l.`key`", "r.`key`"); - String topN = client.topNFunction().orElseThrow().apply("SELECT `key` FROM `test`", - List.of(new JdbcSortItem(column, ASC_NULLS_LAST)), 2); - assertThat(topN).contains("NANVL(`key`, CAST(0 AS " + nativeType + ")) ASC"); + var values = type.createBlockBuilder(null, 2); + if (type.equals(REAL)) { + type.writeLong(values, Float.floatToRawIntBits(Float.NaN)); + type.writeLong(values, Float.floatToRawIntBits(1.0f)); + } else { + type.writeDouble(values, Double.NaN); + type.writeDouble(values, 1.0); + } + var block = values.build(); + for (SortOrder order : SortOrder.values()) { + int comparison = (int) new TypeOperators().getOrderingOperator(type, order, + simpleConvention(FAIL_ON_NULL, BLOCK_POSITION, BLOCK_POSITION)).invokeWithArguments(block, 0, block, 1); + String nanDirection = comparison < 0 ? "DESC" : "ASC"; + String topN = client.topNFunction().orElseThrow().apply("SELECT `key` FROM `test`", + List.of(new JdbcSortItem(column, order)), 2); + assertThat(topN).contains( + "CASE WHEN `key` != `key` THEN 1 ELSE 0 END " + nanDirection, + "NANVL(`key`, CAST(0 AS " + nativeType + ")) " + (order.isAscending() ? "ASC" : "DESC")); + } } } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbPlugin.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbPlugin.java new file mode 100644 index 00000000..8af74c06 --- /dev/null +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbPlugin.java @@ -0,0 +1,27 @@ +package tech.ydb.trino; + +import io.trino.spi.connector.Connector; +import io.trino.testing.TestingConnectorContext; +import org.junit.jupiter.api.Test; + +import java.util.Map; + +import static com.google.common.collect.Iterables.getOnlyElement; +import static io.trino.spi.connector.ConnectorCapabilities.NOT_NULL_COLUMN_CONSTRAINT; +import static org.assertj.core.api.Assertions.assertThat; + +public class TestYdbPlugin { + @Test + public void testProductionConnectorBootstrap() { + Connector connector = getOnlyElement(new YdbPlugin().getConnectorFactories()) + .create("ydb_bootstrap", Map.of("connection-url", "jdbc:ydb:grpc://127.0.0.1:2136/local"), + new TestingConnectorContext()); + try { + assertThat(connector.getPageSinkProvider()).isInstanceOf(YdbPageSinkProvider.class); + assertThat(connector.getCapabilities()).contains(NOT_NULL_COLUMN_CONSTRAINT); + } + finally { + connector.shutdown(); + } + } +} From f90737f8f8c7a6ea56b802fe32a0bcc909790739 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 18:34:30 +0300 Subject: [PATCH 09/13] Record verified YDB connector and upstream handoff evidence --- ydb-trino-adapter/ROADMAP.md | 5 ++-- ydb-trino-adapter/UPSTREAM-READINESS.md | 38 +++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 5 deletions(-) diff --git a/ydb-trino-adapter/ROADMAP.md b/ydb-trino-adapter/ROADMAP.md index 0ff36f75..7c1d8614 100644 --- a/ydb-trino-adapter/ROADMAP.md +++ b/ydb-trino-adapter/ROADMAP.md @@ -130,8 +130,9 @@ unsupported behavior, record: 2. an authoritative documentation link or tracked upstream issue; 3. a focused negative test that proves the connector fails clearly. -The existing negative-date overrides need this treatment. -YDB `Date` starts at the Unix epoch; see +New Trino date columns use signed `Date32`; the connector tests no longer +exclude negative dates. Legacy YDB `Date` starts at the Unix epoch and is +handled separately; see [primitive types](https://ydb.tech/docs/en/yql/reference/types/primitive). CHAR is rejected with the focused inherited contract (`Unsupported column type: char(3)`) because YDB has no fixed-width string diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md index 12d6ee7d..7c667105 100644 --- a/ydb-trino-adapter/UPSTREAM-READINESS.md +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -65,6 +65,11 @@ The separate local Trino checkout preserves follow-up commits: - `cf6e4cf33f`: abort unfinished MERGE sinks on operator close; - `bd9a654ce3`: correct INSERT staging metadata identity; - `dcb449c1f3`: enforce connector limits for unpartitioned MERGE writers. +- `c70d350cf6d2e98d37846f82ff6052d2e780a3c2`: synchronize the reviewed scalar + and write semantics, adapt the 484 sink-provider API, and fix fresh Error + Prone findings. Both implementations now require explicitly + non-transactional MERGE; the engine fixes are preserved separately, not + treated as prerequisites for this batch contract. Its affected core planner/operator group passed 21 tests. Those engine changes are not part of Trino 483 and are not silently assumed here. The current @@ -92,8 +97,7 @@ Java LSP diagnostics were unavailable because jdtls is not installed. Docker's Colima socket was unavailable; Docker/Colima was neither restarted nor repaired. The local full `clean test` attempted 22 entries: 18 passed, 4 integration classes failed in helper setup, with zero errors or skips. -No local integration test completed. Full integration results must come from -the ordinary module CI: +No local integration test completed. The ordinary module CI runs: ```bash mvn -B -ntp -f ydb-trino-adapter/pom.xml clean test @@ -106,7 +110,35 @@ overly broad Date32 predicate fallback, a cached JDBC context-close race, and two H2 reference queries using unsupported typed-literal syntax. The follow-up checks NaN ordering against the actual Trino type operator, pushes only safely bounded Date32 predicates, defaults to uncached JDBC -contexts, and corrects the H2 syntax. Its full real-YDB rerun is still required. +contexts, and corrects the H2 syntax. + +The follow-up [real-YDB CI run 36734257262](https://github.com/ydb-platform/ydb-java-dialects/actions/runs/36734257262) +passed on `2cd8bbede0242ef1a08844e8477f9f5ca02d47e1`: **359 tests, +274 passed, 85 skipped, zero failures or errors**. Class totals are: + +| Group | Tests | Skipped | +| --- | ---: | ---: | +| Inherited connector contract | 273 | 81 | +| Inherited smoke contract | 35 | 4 | +| Production native JOIN matrix | 25 | 0 | +| Production CREATE/INSERT/UPDATE/DELETE/MERGE | 6 | 0 | +| Unit and public plugin-bootstrap tests | 20 | 0 | + +The synchronized local Trino 484 prototype passed a fresh build, style checks, +Error Prone compilation and 14 unit tests: + +```bash +./mvnw -B -ntp -pl plugin/trino-ydb -P errorprone-compiler \ + -Dtest=TestYdbClient,TestYdbColumnMappings,TestYdbMergeSink,TestYdbPushdownSemantics \ + clean verify +``` + +The build retains compiler deprecation warnings for the framework's legacy +JDBC JOIN API and an advisory text-block warning in an existing smoke test; +none are suppressed. Docker-backed tests on Trino 484 and its full upstream +CI matrix remain unverified for this local commit. The green standalone +Trino 483 run does not replace those checks. Upstream CLA and publication +are also left to the designated author. The current PR's exact-head CI and independent review, not the historical prototype or earlier PRs, determine whether this branch is ready to merge. From 8c4e742fc63cb09f49dea48d6ca5edbd55ee52e8 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 19:18:35 +0300 Subject: [PATCH 10/13] Preserve modulo and Timestamp64 boundary semantics 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. --- ydb-trino-adapter/README.md | 13 +++--- ydb-trino-adapter/UPSTREAM-READINESS.md | 18 ++++++-- .../tech/ydb/trino/RewriteDivideModulus.java | 5 ++- .../main/java/tech/ydb/trino/YdbClient.java | 5 ++- .../tech/ydb/trino/YdbColumnMappings.java | 16 ++++++- .../tech/ydb/trino/TestYdbColumnMappings.java | 43 ++++++++++++++++++ .../ydb/trino/TestYdbExpressionRewrites.java | 22 ++++++++++ .../tech/ydb/trino/TestYdbJoinPushdown.java | 44 +++++++++++++++++++ 8 files changed, 154 insertions(+), 12 deletions(-) diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index 0f902163..ba9e20d6 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -117,9 +117,10 @@ Native decimal `NaN` и бесконечности не представимы ## Текст и байты Вычисления целочисленного сложения, вычитания, умножения и отрицания остаются -в Trino: переполнение в YQL не обязано приводить к той же ошибке. Деление +в Trino: переполнение в YQL не обязано приводить к той же ошибке. Деление и остаток передаётся в YDB только для целочисленного результата с ненулевым постоянным -делителем, отличным от `-1`. `strpos` считает позиции Unicode-символов и +делителем, отличным от `-1`: для минимального signed-значения YQL возвращает +NULL при `% -1`, а Trino — ноль. `strpos` считает позиции Unicode-символов и сохраняет SQL `NULL`, а не заменяет его нулём. `trim` остаётся в Trino: byte-oriented `String::Strip` не заменяет Unicode trim. @@ -139,9 +140,11 @@ YDB `Text` отображается в Trino как `varchar`, а `Bytes` — к `Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp`/`Timestamp64` — `Instant` с микросекундами. В построчных UPDATE/DELETE собственный merge sink использует исходный `JdbcTypeHandle` и те же native write mappings, что и обычная запись. -Date32-предикаты передаются в YDB только с границами внутри документированного -диапазона. Предикаты с выходящими за него значениями, а также предикаты -для legacy temporal типов остаются в Trino. +Date32- и Timestamp64-предикаты передаются в YDB только с представимыми +границами. Диапазон Timestamp64 в микросекундах от эпохи: +`[-4611669897600000000, 4611669811199999999]`. +Предикаты с выходящими за него значениями остаются в Trino, а запись таких +значений отклоняется. Предикаты для legacy temporal типов также остаются в Trino. ## JDBC-контексты diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md index 7c667105..9ee5f4af 100644 --- a/ydb-trino-adapter/UPSTREAM-READINESS.md +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -70,6 +70,9 @@ The separate local Trino checkout preserves follow-up commits: Prone findings. Both implementations now require explicitly non-transactional MERGE; the engine fixes are preserved separately, not treated as prerequisites for this batch contract. +- `81f4a2b47eb50396f5176cca15a1ed3b215b39d1`: correct modulo and extended + timestamp boundaries found by the independent gate; fresh source archives + and all 21 local unit/API tests were rebuilt successfully. Its affected core planner/operator group passed 21 tests. Those engine changes are not part of Trino 483 and are not silently assumed here. The current @@ -83,7 +86,7 @@ no replacement upstream PR is created by this work. ## Validation -With Temurin 25.0.2, the following standalone command passed 20 tests, with +With Temurin 25.0.2, the following standalone command passed 24 tests, with zero failures, errors or skips, and built the module: ```bash @@ -125,11 +128,11 @@ passed on `2cd8bbede0242ef1a08844e8477f9f5ca02d47e1`: **359 tests, | Unit and public plugin-bootstrap tests | 20 | 0 | The synchronized local Trino 484 prototype passed a fresh build, style checks, -Error Prone compilation and 14 unit tests: +Error Prone compilation and 21 unit/API tests: ```bash ./mvnw -B -ntp -pl plugin/trino-ydb -P errorprone-compiler \ - -Dtest=TestYdbClient,TestYdbColumnMappings,TestYdbMergeSink,TestYdbPushdownSemantics \ + -Dtest=TestYdbClient,TestYdbColumnMappings,TestYdbMergeSink,TestYdbPushdownSemantics,TestYdbJoinCondition,TestYdbPlugin \ clean verify ``` @@ -140,5 +143,14 @@ CI matrix remain unverified for this local commit. The green standalone Trino 483 run does not replace those checks. Upstream CLA and publication are also left to the designated author. +The independent gate of `f90737f` / `c70d350cf6` then found two additional +semantic boundaries not covered by those green runs: signed-minimum modulo +`-1` produces YQL NULL instead of Trino zero, and Timestamp64 predicates +could bind values outside the native range. Both implementations now keep +modulo by `-1` in Trino and guard Timestamp64 domains, expression constants, +and writes. Regressions cover filter retention, projection/JOIN results, +native endpoints, just-outside bounds, NULL and unbounded predicates. +The subsequent exact-head CI and delta review must cover these additions. + The current PR's exact-head CI and independent review, not the historical prototype or earlier PRs, determine whether this branch is ready to merge. diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java index 6e05ca05..ee251b62 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/RewriteDivideModulus.java @@ -23,10 +23,11 @@ /** * Rewrite $divide(a, b), $modulus(a, b) as a / b, a % b, - * for integral operands with a non-zero constant divisor, excluding division by -1. + * for integral operands with a non-zero constant divisor other than -1. *

* This is because YDB, unlike Trino, suppresses arithmetic errors (including division by zero), and any * non-constant expression may evaluate to zero and cause a different result if pushed down to YDB. + * YQL also returns NULL for signed-minimum modulo -1, while Trino returns zero. */ public class RewriteDivideModulus implements ConnectorExpressionRule { private final Pattern PATTERN; @@ -55,7 +56,7 @@ public Optional rewrite(Call call, Captures captures, R () -> call.getArguments().get(1) instanceof Constant rightConstant && rightConstant.getValue() instanceof Number number && number.longValue() != 0 && - (!call.getFunctionName().equals(DIVIDE_FUNCTION_NAME) || number.longValue() != -1), + number.longValue() != -1, (left, right) -> format("(%s) %s (%s)", left, operator, right) ); } diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java index 893434c6..bcb56bec 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClient.java @@ -56,6 +56,7 @@ import io.trino.spi.expression.Constant; import io.trino.spi.expression.Variable; import io.trino.spi.type.DecimalType; +import io.trino.spi.type.TimestampType; import io.trino.spi.type.Type; import io.trino.spi.type.VarcharType; @@ -243,7 +244,9 @@ private boolean hasSupportedValueMapping(JdbcColumnHandle column) { private boolean supportsExpression(ConnectorExpression expression, Map assignments) { if (expression instanceof Constant constant) { - return YdbTypeUtils.toTypeHandle(constant.getType()).isPresent(); + return YdbTypeUtils.toTypeHandle(constant.getType()).isPresent() && + (!(constant.getType() instanceof TimestampType) || constant.getValue() == null || + YdbColumnMappings.isTimestamp64ValueSupported((long) constant.getValue())); } if (expression instanceof Variable variable) { JdbcColumnHandle column = (JdbcColumnHandle) assignments.get(variable.getName()); diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java index 4f3c2552..7485d7e1 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java @@ -87,7 +87,18 @@ static ColumnMapping timestampColumnMapping(JdbcTypeHandle typeHandle) { return Math.addExact(Math.multiplyExact(instant.getEpochSecond(), 1_000_000), instant.getNano() / 1_000); }, writeFunction, - typeName.equalsIgnoreCase("Timestamp64") ? FULL_PUSHDOWN : DISABLE_PUSHDOWN); + (session, domain) -> { + boolean safeBounds = typeName.equalsIgnoreCase("Timestamp64") && + domain.getValues().getRanges().getOrderedRanges().stream().allMatch(range -> + (range.isLowUnbounded() || isTimestamp64ValueSupported((long) range.getLowBoundedValue())) && + (range.isHighUnbounded() || isTimestamp64ValueSupported((long) range.getHighBoundedValue()))); + return (safeBounds ? FULL_PUSHDOWN : DISABLE_PUSHDOWN).apply(session, domain); + }); + } + + static boolean isTimestamp64ValueSupported(long value) { + // YDB yql/essentials/public/udf/udf_data_type.h: MIN_TIMESTAMP64 / MAX_TIMESTAMP64. + return value >= -4611669897600000000L && value <= 4611669811199999999L; } static ColumnMapping unsignedColumnMapping(Type type, int typeOffset) { @@ -185,6 +196,9 @@ static LongWriteFunction timestampWriteFunction(int sqlType) { if (sqlType == YDB_TIMESTAMP_SQL_TYPE && value < 0) { throw new SQLDataException("Value is outside the YDB Timestamp storage range"); } + if (sqlType == YDB_TIMESTAMP64_SQL_TYPE && !isTimestamp64ValueSupported(value)) { + throw new SQLDataException("Value is outside the YDB Timestamp64 range"); + } statement.setObject(index, fromTrinoTimestamp(value).toInstant(UTC), sqlType); }); } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java index e3797342..555c14b6 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java @@ -37,6 +37,7 @@ import static io.trino.spi.type.DecimalType.createDecimalType; import static io.trino.spi.type.DateType.DATE; import static io.trino.spi.type.SmallintType.SMALLINT; +import static io.trino.spi.type.TimestampType.TIMESTAMP_MICROS; import static io.trino.testing.TestingConnectorSession.SESSION; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; @@ -146,6 +147,48 @@ public void testDate32PredicateBounds() { } } + @Test + public void testTimestamp64PredicateBounds() { + var session = TestingConnectorSession.builder().setPropertyMetadata( + new JdbcMetadataSessionProperties(new JdbcMetadataConfig(), Optional.empty()).getSessionProperties()).build(); + JdbcTypeHandle handle = YdbTypeUtils.toTypeHandle(TIMESTAMP_MICROS).orElseThrow(); + var controller = YdbColumnMappings.timestampColumnMapping(handle).getPredicatePushdownController(); + long minimum = -4611669897600000000L; + long maximum = 4611669811199999999L; + for (Domain domain : List.of(Domain.all(TIMESTAMP_MICROS), Domain.onlyNull(TIMESTAMP_MICROS), + Domain.singleValue(TIMESTAMP_MICROS, minimum), Domain.singleValue(TIMESTAMP_MICROS, maximum), + Domain.create(ValueSet.ofRanges(Range.greaterThanOrEqual(TIMESTAMP_MICROS, minimum)), false), + Domain.create(ValueSet.ofRanges(Range.lessThanOrEqual(TIMESTAMP_MICROS, maximum)), true))) { + var result = controller.apply(session, domain); + assertThat(result.getPushedDown()).isEqualTo(domain); + assertThat(result.getRemainingFilter().isAll()).isTrue(); + } + for (Domain domain : List.of(Domain.singleValue(TIMESTAMP_MICROS, minimum - 1), + Domain.singleValue(TIMESTAMP_MICROS, maximum + 1), + Domain.create(ValueSet.ofRanges(Range.lessThan(TIMESTAMP_MICROS, maximum + 1)), false), + Domain.create(ValueSet.ofRanges(Range.greaterThan(TIMESTAMP_MICROS, minimum - 1)), true))) { + var result = controller.apply(session, domain); + assertThat(result.getPushedDown().isAll()).isTrue(); + assertThat(result.getRemainingFilter()).isEqualTo(domain); + } + } + + @Test + public void testTimestamp64WriterBounds() throws Exception { + LongWriteFunction writer = YdbColumnMappings.timestampWriteFunction(SQL_KIND_PRIMITIVE + 27); + List> calls = new ArrayList<>(); + PreparedStatement statement = statement(calls); + writer.set(statement, 1, -4611669897600000000L); + writer.set(statement, 2, 4611669811199999999L); + writer.setNull(statement, 3); + assertThat(calls).containsExactly( + List.of(1, Instant.ofEpochSecond(-4611669897600L), SQL_KIND_PRIMITIVE + 27), + List.of(2, Instant.ofEpochSecond(4611669811199L, 999999000), SQL_KIND_PRIMITIVE + 27), + List.of(3, SQL_KIND_PRIMITIVE + 27)); + assertThatThrownBy(() -> writer.set(statement, 1, -4611669897600000001L)).isInstanceOf(SQLDataException.class); + assertThatThrownBy(() -> writer.set(statement, 1, 4611669811200000000L)).isInstanceOf(SQLDataException.class); + } + @Test public void testDatetimeRejectsFractionalSeconds() { JdbcTypeHandle handle = new JdbcTypeHandle(Types.TIMESTAMP, Optional.of("Datetime64"), diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java index a740b3f2..cd49145e 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbExpressionRewrites.java @@ -11,6 +11,7 @@ import io.trino.spi.expression.Constant; import io.trino.spi.expression.FunctionName; import io.trino.spi.expression.Variable; +import io.trino.type.BigintOperators; import org.junit.jupiter.api.Test; import java.sql.SQLException; @@ -21,8 +22,12 @@ import static io.airlift.slice.Slices.utf8Slice; import static io.trino.spi.expression.StandardFunctions.ADD_FUNCTION_NAME; import static io.trino.spi.expression.StandardFunctions.DIVIDE_FUNCTION_NAME; +import static io.trino.spi.expression.StandardFunctions.LESS_THAN_OPERATOR_FUNCTION_NAME; +import static io.trino.spi.expression.StandardFunctions.MODULO_FUNCTION_NAME; import static io.trino.spi.expression.StandardFunctions.NULLIF_FUNCTION_NAME; import static io.trino.spi.type.BigintType.BIGINT; +import static io.trino.spi.type.BooleanType.BOOLEAN; +import static io.trino.spi.type.TimestampType.TIMESTAMP_MICROS; import static io.trino.spi.type.VarcharType.VARCHAR; import static io.trino.testing.TestingConnectorSession.SESSION; import static org.assertj.core.api.Assertions.assertThat; @@ -70,6 +75,23 @@ public void testUnicodeTrimIsNotPushedDown() { new Call(VARCHAR, new FunctionName("trim"), List.of(new Variable("v", VARCHAR))), Map.of("v", column))).isEmpty(); } + @Test + public void testSignedMinimumModuloFallsBack() { + assertThat(BigintOperators.modulo(Long.MIN_VALUE, -1)).isZero(); + JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(BIGINT).orElseThrow(), BIGINT); + assertThat(client.convertPredicate(SESSION, new Call(BIGINT, MODULO_FUNCTION_NAME, List.of( + new Variable("v", BIGINT), new Constant(-1L, BIGINT))), Map.of("v", column))).isEmpty(); + } + + @Test + public void testTimestamp64ExpressionBounds() { + JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(TIMESTAMP_MICROS).orElseThrow(), TIMESTAMP_MICROS); + for (long bound : List.of(-4611669897600000001L, 4611669811200000000L)) { + assertThat(client.convertPredicate(SESSION, new Call(BOOLEAN, LESS_THAN_OPERATOR_FUNCTION_NAME, List.of( + new Variable("v", TIMESTAMP_MICROS), new Constant(bound, TIMESTAMP_MICROS))), Map.of("v", column))).isEmpty(); + } + } + @Test public void testOverflowingArithmeticIsNotPushedDown() { JdbcColumnHandle column = new JdbcColumnHandle("value", YdbTypeUtils.toTypeHandle(BIGINT).orElseThrow(), BIGINT); diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java index 2efa0df3..a9dfa88f 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java @@ -13,11 +13,16 @@ import org.junit.jupiter.params.provider.MethodSource; import tech.ydb.test.junit5.YdbHelperExtension; +import java.time.LocalDateTime; +import java.time.format.DateTimeFormatter; import java.util.List; +import java.util.Locale; import java.util.stream.IntStream; import java.util.stream.Stream; import static io.trino.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE; +import static io.trino.type.DateTimes.parseTimestamp; +import static java.time.ZoneOffset.UTC; import static java.util.stream.Collectors.joining; import static org.assertj.core.api.Assertions.assertThat; import static org.junit.jupiter.params.provider.Arguments.arguments; @@ -133,9 +138,48 @@ public void testOverflowIsEvaluatedByTrino() { assertThat(query(joinSession(), join + "l.k + 1 = r.k")).failure().hasErrorCode(NUMERIC_VALUE_OUT_OF_RANGE); assertThat(query(joinSession(), join + "l.k / -1 = r.k")).failure().hasErrorCode(NUMERIC_VALUE_OUT_OF_RANGE); assertThat(query(joinSession(), join + "CAST(l.k AS SMALLINT) = r.k")).failure().hasErrorCode(NUMERIC_VALUE_OUT_OF_RANGE); + assertThat(query("SELECT id, k % BIGINT '-1' FROM " + left.getName())) + .matches("VALUES (BIGINT '1', BIGINT '0'), (2, 0)"); + assertThat(query("SELECT id FROM " + left.getName() + " WHERE k % BIGINT '-1' = 0")) + .matches("VALUES BIGINT '1', BIGINT '2'"); + assertThat(query(joinSession(), join + "l.k % BIGINT '-1' = r.k")) + .matches("VALUES BIGINT '1', BIGINT '2'") + .joinIsNotFullyPushedDown(); } } + @Test + public void testTimestamp64PredicateBoundaries() { + long minimum = -4611669897600000000L; + long maximum = 4611669811199999999L; + JdbcSqlExecutor executor = new JdbcSqlExecutor(YdbQueryRunner.buildJdbcUrl(ydb)); + try (TestTable table = nativeTable(executor, "timestamp_bounds_", "(id Int64, k Timestamp64, PRIMARY KEY (id))", "id, k", + List.of("1, CAST(" + minimum + "l AS Timestamp64)", "2, CAST(0l AS Timestamp64)", + "3, CAST(" + maximum + "l AS Timestamp64)", "4, NULL"))) { + String select = "SELECT id FROM " + table.getName(); + assertThat(query(select + " WHERE k = " + timestampLiteral(minimum))).matches("VALUES BIGINT '1'").isFullyPushedDown(); + assertThat(query(select + " WHERE k = " + timestampLiteral(maximum))).matches("VALUES BIGINT '3'").isFullyPushedDown(); + assertThat(query(select + " WHERE k >= " + timestampLiteral(minimum))).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'").isFullyPushedDown(); + assertThat(query(select + " WHERE k <= " + timestampLiteral(maximum))).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'").isFullyPushedDown(); + assertThat(query(select + " WHERE k IS NULL")).matches("VALUES BIGINT '4'").isFullyPushedDown(); + assertThat(query(select + " WHERE k IS NOT NULL")).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'").isFullyPushedDown(); + assertThat(query(select + " WHERE k < " + timestampLiteral(maximum + 1))).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'"); + assertThat(query(select + " WHERE k > " + timestampLiteral(minimum - 1))).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'"); + assertThat(query(select + " WHERE k = " + timestampLiteral(maximum + 1))).returnsEmptyResult(); + assertThat(query(select + " WHERE k = " + timestampLiteral(minimum - 1))).returnsEmptyResult(); + assertThat(query(select + " WHERE k < " + timestampLiteral(maximum + 1) + " OR id = 4")) + .matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3', BIGINT '4'"); + } + } + + private static String timestampLiteral(long micros) { + String value = LocalDateTime.ofEpochSecond(Math.floorDiv(micros, 1_000_000), + (int) Math.floorMod(micros, 1_000_000) * 1_000, UTC) + .format(DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss.SSSSSS", Locale.ROOT)); + assertThat(parseTimestamp(6, value)).isEqualTo(micros); + return "TIMESTAMP '" + value + "'"; + } + private Session joinSession() { return Session.builder(getSession()).setCatalogSessionProperty("local", "join_pushdown_enabled", "true").build(); } From c5d5cf9d4b92018a76c8b4e36b270b2f1f6b6028 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 19:48:58 +0300 Subject: [PATCH 11/13] Bind Timestamp64 endpoints without the SDK range off-by-one 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. --- ydb-trino-adapter/README.md | 8 ++++-- ydb-trino-adapter/UPSTREAM-READINESS.md | 4 +++ .../tech/ydb/trino/YdbColumnMappings.java | 26 ++++++++++++++++--- .../tech/ydb/trino/TestYdbColumnMappings.java | 9 ++++--- .../tech/ydb/trino/TestYdbJoinPushdown.java | 12 +++++++++ 5 files changed, 50 insertions(+), 9 deletions(-) diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index ba9e20d6..c975a470 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -136,8 +136,12 @@ YDB `Text` отображается в Trino как `varchar`, а `Bytes` — к исходная точность `timestamp(3)` в метаданных не сохраняется. Чтение timestamps использует UTC, а не часовой пояс JVM. Запись дробных секунд в `Datetime`/`Datetime64` отклоняется без молчаливого усечения. -При записи адаптер передаёт YDB JDBC точный vendor type по `TYPE_NAME` и больше не задаёт `forceSignedDatetimes`: -`Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp`/`Timestamp64` — `Instant` с микросекундами. +При записи адаптер сохраняет native-тип из `TYPE_NAME` и не задаёт `forceSignedDatetimes`: +`Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp` — `Instant` с микросекундами. +Для `Timestamp64` передаются Int64-микросекунды с параметризованным +`CAST(? AS Timestamp64)`: SDK 2.4.10 ошибочно исключает саму допустимую верхнюю +границу в фабрике `newTimestamp64`. Явный YQL CAST сохраняет эту границу без +встраивания значений в SQL. В построчных UPDATE/DELETE собственный merge sink использует исходный `JdbcTypeHandle` и те же native write mappings, что и обычная запись. Date32- и Timestamp64-предикаты передаются в YDB только с представимыми diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md index 9ee5f4af..3186b63a 100644 --- a/ydb-trino-adapter/UPSTREAM-READINESS.md +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -73,6 +73,10 @@ The separate local Trino checkout preserves follow-up commits: - `81f4a2b47eb50396f5176cca15a1ed3b215b39d1`: correct modulo and extended timestamp boundaries found by the independent gate; fresh source archives and all 21 local unit/API tests were rebuilt successfully. +- `c4041e10d8e2bc53c980e4cd7a46d9dd6a8264e9`: bind Timestamp64 with an Int64 + parameter and structured YQL CAST, avoiding SDK 2.4.10's rejection of the + inclusive maximum. The expanded live fixture also checks nullable and + NOT NULL endpoint writes; all 21 local unit/API tests pass. Its affected core planner/operator group passed 21 tests. Those engine changes are not part of Trino 483 and are not silently assumed here. The current diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java index 7485d7e1..53e1b59a 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java @@ -14,6 +14,7 @@ import java.sql.ResultSet; import java.sql.SQLDataException; import java.sql.SQLException; +import java.sql.Types; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -190,15 +191,34 @@ private static LongWriteFunction datetimeWriteFunction(int sqlType) { } static LongWriteFunction timestampWriteFunction(int sqlType) { + if (sqlType == YDB_TIMESTAMP64_SQL_TYPE) { + // SDK 2.4.10 incorrectly rejects the inclusive maximum in newTimestamp64. + return new LongWriteFunction() { + @Override + public String getBindExpression() { + return "CAST(? AS Timestamp64)"; + } + + @Override + public void set(PreparedStatement statement, int index, long value) throws SQLException { + if (!isTimestamp64ValueSupported(value)) { + throw new SQLDataException("Value is outside the YDB Timestamp64 range"); + } + statement.setLong(index, value); + } + + @Override + public void setNull(PreparedStatement statement, int index) throws SQLException { + statement.setNull(index, Types.BIGINT); + } + }; + } return LongWriteFunction.of( sqlType, (statement, index, value) -> { if (sqlType == YDB_TIMESTAMP_SQL_TYPE && value < 0) { throw new SQLDataException("Value is outside the YDB Timestamp storage range"); } - if (sqlType == YDB_TIMESTAMP64_SQL_TYPE && !isTimestamp64ValueSupported(value)) { - throw new SQLDataException("Value is outside the YDB Timestamp64 range"); - } statement.setObject(index, fromTrinoTimestamp(value).toInstant(UTC), sqlType); }); } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java index 555c14b6..68a3b1a4 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java @@ -176,15 +176,16 @@ public void testTimestamp64PredicateBounds() { @Test public void testTimestamp64WriterBounds() throws Exception { LongWriteFunction writer = YdbColumnMappings.timestampWriteFunction(SQL_KIND_PRIMITIVE + 27); + assertThat(writer.getBindExpression()).isEqualTo("CAST(? AS Timestamp64)"); List> calls = new ArrayList<>(); PreparedStatement statement = statement(calls); writer.set(statement, 1, -4611669897600000000L); writer.set(statement, 2, 4611669811199999999L); writer.setNull(statement, 3); assertThat(calls).containsExactly( - List.of(1, Instant.ofEpochSecond(-4611669897600L), SQL_KIND_PRIMITIVE + 27), - List.of(2, Instant.ofEpochSecond(4611669811199L, 999999000), SQL_KIND_PRIMITIVE + 27), - List.of(3, SQL_KIND_PRIMITIVE + 27)); + List.of(1, -4611669897600000000L), + List.of(2, 4611669811199999999L), + List.of(3, Types.BIGINT)); assertThatThrownBy(() -> writer.set(statement, 1, -4611669897600000001L)).isInstanceOf(SQLDataException.class); assertThatThrownBy(() -> writer.set(statement, 1, 4611669811200000000L)).isInstanceOf(SQLDataException.class); } @@ -200,7 +201,7 @@ public void testDatetimeRejectsFractionalSeconds() { private static PreparedStatement statement(List> calls) { return (PreparedStatement) Proxy.newProxyInstance(PreparedStatement.class.getClassLoader(), new Class[]{PreparedStatement.class}, (_, method, args) -> { - assertThat(method.getName()).isIn("setObject", "setNull"); + assertThat(method.getName()).isIn("setObject", "setLong", "setNull"); calls.add(Arrays.asList(args)); return null; }); diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java index a9dfa88f..69b3c4fd 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java @@ -169,6 +169,18 @@ public void testTimestamp64PredicateBoundaries() { assertThat(query(select + " WHERE k = " + timestampLiteral(minimum - 1))).returnsEmptyResult(); assertThat(query(select + " WHERE k < " + timestampLiteral(maximum + 1) + " OR id = 4")) .matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3', BIGINT '4'"); + assertUpdate("INSERT INTO " + table.getName() + " VALUES (5, NULL), (6, " + timestampLiteral(maximum) + + "), (7, " + timestampLiteral(minimum) + ")", 3); + assertThat(query("SELECT id, k IS NULL FROM " + table.getName() + " WHERE id >= 5")) + .matches("VALUES (BIGINT '5', true), (6, false), (7, false)"); + assertThat(query(select + " WHERE id = 6 AND k = " + timestampLiteral(maximum))).matches("VALUES BIGINT '6'"); + } + try (TestTable table = new TestTable(getQueryRunner()::execute, "timestamp_notnull_", + "(id bigint, k timestamp(6) NOT NULL) WITH (primary_key = ARRAY['id'])")) { + assertUpdate("INSERT INTO " + table.getName() + " VALUES (1, " + timestampLiteral(maximum) + + "), (2, " + timestampLiteral(minimum) + ")", 2); + assertThat(query("SELECT id FROM " + table.getName() + " WHERE k = " + timestampLiteral(maximum))) + .matches("VALUES BIGINT '1'"); } } From 6fe4de5a7c0ab0aaf2686dfad894c021bf22c0a1 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 20:09:57 +0300 Subject: [PATCH 12/13] Preserve native Timestamp64 binding for NOT NULL columns 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. --- ydb-trino-adapter/README.md | 8 +-- ydb-trino-adapter/UPSTREAM-READINESS.md | 10 ++-- .../tech/ydb/trino/YdbColumnMappings.java | 56 +++++++++++-------- .../tech/ydb/trino/TestYdbColumnMappings.java | 29 ++++++++-- 4 files changed, 65 insertions(+), 38 deletions(-) diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index c975a470..b0ee7e20 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -138,10 +138,10 @@ YDB `Text` отображается в Trino как `varchar`, а `Bytes` — к в `Datetime`/`Datetime64` отклоняется без молчаливого усечения. При записи адаптер сохраняет native-тип из `TYPE_NAME` и не задаёт `forceSignedDatetimes`: `Date`/`Date32` получают дни от эпохи, `Datetime`/`Datetime64` — секунды UTC, а `Timestamp` — `Instant` с микросекундами. -Для `Timestamp64` передаются Int64-микросекунды с параметризованным -`CAST(? AS Timestamp64)`: SDK 2.4.10 ошибочно исключает саму допустимую верхнюю -границу в фабрике `newTimestamp64`. Явный YQL CAST сохраняет эту границу без -встраивания значений в SQL. +Для `Timestamp64` передаются типизированные SDK-значения. SDK 2.4.10 ошибочно +исключает допустимую верхнюю границу в фабрике `newTimestamp64`; для этой +точки адаптер сохраняет native-тип и Int64-микросекунды непосредственно +в протокольном значении. Это не добавляет Optional к NOT NULL столбцам. В построчных UPDATE/DELETE собственный merge sink использует исходный `JdbcTypeHandle` и те же native write mappings, что и обычная запись. Date32- и Timestamp64-предикаты передаются в YDB только с представимыми diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md index 3186b63a..b907e02b 100644 --- a/ydb-trino-adapter/UPSTREAM-READINESS.md +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -73,10 +73,12 @@ The separate local Trino checkout preserves follow-up commits: - `81f4a2b47eb50396f5176cca15a1ed3b215b39d1`: correct modulo and extended timestamp boundaries found by the independent gate; fresh source archives and all 21 local unit/API tests were rebuilt successfully. -- `c4041e10d8e2bc53c980e4cd7a46d9dd6a8264e9`: bind Timestamp64 with an Int64 - parameter and structured YQL CAST, avoiding SDK 2.4.10's rejection of the - inclusive maximum. The expanded live fixture also checks nullable and - NOT NULL endpoint writes; all 21 local unit/API tests pass. +- `c4041e10d8`: the intermediate SQL CAST solution passed unit tests but the + added live NOT NULL write exposed YQL's Optional result type. +- `fe21afac93f670cc545aeb9b0f25644b62053d92`: use the driver's native SDK-value + path instead. The inclusive endpoint is encoded with the Timestamp64 type, + not an optional CAST. Tests now exercise real driver required/optional value + conversion, not only recorded setter arguments; all 21 unit/API tests pass. Its affected core planner/operator group passed 21 tests. Those engine changes are not part of Trino 483 and are not silently assumed here. The current diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java index 53e1b59a..f007448f 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbColumnMappings.java @@ -8,13 +8,15 @@ import io.trino.spi.type.DecimalType; import io.trino.spi.type.Int128; import io.trino.spi.type.Type; +import tech.ydb.proto.ValueProtos; +import tech.ydb.table.values.PrimitiveType; +import tech.ydb.table.values.PrimitiveValue; import java.math.BigDecimal; import java.sql.PreparedStatement; import java.sql.ResultSet; import java.sql.SQLDataException; import java.sql.SQLException; -import java.sql.Types; import java.time.Instant; import java.time.LocalDate; import java.time.LocalDateTime; @@ -191,35 +193,41 @@ private static LongWriteFunction datetimeWriteFunction(int sqlType) { } static LongWriteFunction timestampWriteFunction(int sqlType) { - if (sqlType == YDB_TIMESTAMP64_SQL_TYPE) { - // SDK 2.4.10 incorrectly rejects the inclusive maximum in newTimestamp64. - return new LongWriteFunction() { - @Override - public String getBindExpression() { - return "CAST(? AS Timestamp64)"; - } - - @Override - public void set(PreparedStatement statement, int index, long value) throws SQLException { - if (!isTimestamp64ValueSupported(value)) { - throw new SQLDataException("Value is outside the YDB Timestamp64 range"); - } - statement.setLong(index, value); - } - - @Override - public void setNull(PreparedStatement statement, int index) throws SQLException { - statement.setNull(index, Types.BIGINT); - } - }; - } return LongWriteFunction.of( sqlType, (statement, index, value) -> { if (sqlType == YDB_TIMESTAMP_SQL_TYPE && value < 0) { throw new SQLDataException("Value is outside the YDB Timestamp storage range"); } - statement.setObject(index, fromTrinoTimestamp(value).toInstant(UTC), sqlType); + if (sqlType != YDB_TIMESTAMP64_SQL_TYPE) { + statement.setObject(index, fromTrinoTimestamp(value).toInstant(UTC), sqlType); + return; + } + if (!isTimestamp64ValueSupported(value)) { + throw new SQLDataException("Value is outside the YDB Timestamp64 range"); + } + if (value != 4611669811199999999L) { + statement.setObject(index, PrimitiveValue.newTimestamp64(value), sqlType); + return; + } + // SDK 2.4.10 rejects this inclusive endpoint. Preserve the native type; + // SQL CAST would introduce Optional and break NOT NULL writes. + statement.setObject(index, new PrimitiveValue() { + @Override + public PrimitiveType getType() { + return PrimitiveType.Timestamp64; + } + + @Override + public Instant getTimestamp64() { + return fromTrinoTimestamp(value).toInstant(UTC); + } + + @Override + public ValueProtos.Value toPb() { + return PrimitiveValue.newInt64(value).toPb(); + } + }, sqlType); }); } } diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java index 68a3b1a4..33c8202e 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbColumnMappings.java @@ -17,6 +17,11 @@ import io.trino.spi.type.Int128; import io.trino.testing.TestingConnectorSession; import org.junit.jupiter.api.Test; +import tech.ydb.jdbc.common.YdbTypes; +import tech.ydb.jdbc.query.params.ValueFactory; +import tech.ydb.table.values.DecimalType; +import tech.ydb.table.values.PrimitiveType; +import tech.ydb.table.values.PrimitiveValue; import java.lang.reflect.Proxy; import java.math.BigDecimal; @@ -176,16 +181,28 @@ public void testTimestamp64PredicateBounds() { @Test public void testTimestamp64WriterBounds() throws Exception { LongWriteFunction writer = YdbColumnMappings.timestampWriteFunction(SQL_KIND_PRIMITIVE + 27); - assertThat(writer.getBindExpression()).isEqualTo("CAST(? AS Timestamp64)"); + assertThat(writer.getBindExpression()).isEqualTo("?"); List> calls = new ArrayList<>(); PreparedStatement statement = statement(calls); writer.set(statement, 1, -4611669897600000000L); writer.set(statement, 2, 4611669811199999999L); writer.setNull(statement, 3); - assertThat(calls).containsExactly( - List.of(1, -4611669897600000000L), - List.of(2, 4611669811199999999L), - List.of(3, Types.BIGINT)); + assertThat(calls).hasSize(3); + YdbTypes types = new YdbTypes(false, DecimalType.getDefault()); + long[] boundaries = {-4611669897600000000L, 4611669811199999999L}; + for (int index = 0; index < boundaries.length; index++) { + assertThat(calls.get(index).get(0)).isEqualTo(index + 1); + assertThat(calls.get(index).get(2)).isEqualTo(SQL_KIND_PRIMITIVE + 27); + PrimitiveValue value = (PrimitiveValue) calls.get(index).get(1); + var required = ValueFactory.readValue("$timestamp", value, types.find(PrimitiveType.Timestamp64)); + var optional = ValueFactory.readValue("$timestamp", value, types.find(PrimitiveType.Timestamp64.makeOptional())); + assertThat(required.getType()).isEqualTo(PrimitiveType.Timestamp64); + assertThat(required.toPb().getInt64Value()).isEqualTo(boundaries[index]); + assertThat(optional.asOptional().get().toPb().getInt64Value()).isEqualTo(boundaries[index]); + } + assertThat(calls.get(2)).containsExactly(3, SQL_KIND_PRIMITIVE + 27); + assertThat(ValueFactory.readValue("$timestamp", null, types.find(PrimitiveType.Timestamp64.makeOptional())) + .asOptional().isPresent()).isFalse(); assertThatThrownBy(() -> writer.set(statement, 1, -4611669897600000001L)).isInstanceOf(SQLDataException.class); assertThatThrownBy(() -> writer.set(statement, 1, 4611669811200000000L)).isInstanceOf(SQLDataException.class); } @@ -201,7 +218,7 @@ public void testDatetimeRejectsFractionalSeconds() { private static PreparedStatement statement(List> calls) { return (PreparedStatement) Proxy.newProxyInstance(PreparedStatement.class.getClassLoader(), new Class[]{PreparedStatement.class}, (_, method, args) -> { - assertThat(method.getName()).isIn("setObject", "setLong", "setNull"); + assertThat(method.getName()).isIn("setObject", "setNull"); calls.add(Arrays.asList(args)); return null; }); From 2a443c1909b244e2d102d01414567f6ef9f17ed3 Mon Sep 17 00:00:00 2001 From: KirillKurdyukov Date: Wed, 30 Sep 2026 20:23:08 +0300 Subject: [PATCH 13/13] Keep native Timestamp64 parameters intact in IN predicates 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. --- ydb-trino-adapter/README.md | 4 ++++ ydb-trino-adapter/UPSTREAM-READINESS.md | 4 ++++ .../src/main/java/tech/ydb/trino/YdbClientModule.java | 2 ++ .../src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java | 2 ++ 4 files changed, 12 insertions(+) diff --git a/ydb-trino-adapter/README.md b/ydb-trino-adapter/README.md index b0ee7e20..38a449d0 100644 --- a/ydb-trino-adapter/README.md +++ b/ydb-trino-adapter/README.md @@ -157,3 +157,7 @@ Date32- и Timestamp64-предикаты передаются в YDB тольк закрываемый контекст. Каждый JDBC connection поэтому владеет своим контекстом. Это не retry и не replay записей. Параметры JDBC URL имеют приоритет над properties; не включайте `cacheConnectionsInDriver=true` на этой версии драйвера. +Также задаётся `replaceJdbcInByYqlList=false`: оптимизированный параметр списка +в JDBC 2.4.1 не поддерживает типизированные SDK-значения Timestamp64. +Предикаты `IN` продолжают передаваться в YDB с отдельными параметрами. +Не включайте `replaceJdbcInByYqlList=true` в JDBC URL на этой версии драйвера. diff --git a/ydb-trino-adapter/UPSTREAM-READINESS.md b/ydb-trino-adapter/UPSTREAM-READINESS.md index b907e02b..85a13e87 100644 --- a/ydb-trino-adapter/UPSTREAM-READINESS.md +++ b/ydb-trino-adapter/UPSTREAM-READINESS.md @@ -79,6 +79,10 @@ The separate local Trino checkout preserves follow-up commits: path instead. The inclusive endpoint is encoded with the Timestamp64 type, not an optional CAST. Tests now exercise real driver required/optional value conversion, not only recorded setter arguments; all 21 unit/API tests pass. +- `dce9fe0f112e2341d645ed8282dbc61173e78864`: retain scalar parameters for + `IN`, avoiding JDBC 2.4.1's list rewrite that bypasses native SDK values. + Pushdown stays enabled and the live test covers both Timestamp64 endpoints. + Fresh Error Prone, packaging and all 21 unit/API tests pass. Its affected core planner/operator group passed 21 tests. Those engine changes are not part of Trino 483 and are not silently assumed here. The current diff --git a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java index 02696bb0..dbafcddc 100644 --- a/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java +++ b/ydb-trino-adapter/src/main/java/tech/ydb/trino/YdbClientModule.java @@ -66,6 +66,8 @@ public static ConnectionFactory createConnectionFactory( // JDBC 2.4.1 can close a cached context between lookup and connection registration. Properties properties = new Properties(); properties.setProperty("cacheConnectionsInDriver", "false"); + // InListJdbcPrm bypasses the driver's native SDK-value binding path. + properties.setProperty("replaceJdbcInByYqlList", "false"); return DriverConnectionFactory.builder( new YdbDriver(), config.getConnectionUrl(), diff --git a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java index 69b3c4fd..b971a3dd 100644 --- a/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java +++ b/ydb-trino-adapter/src/test/java/tech/ydb/trino/TestYdbJoinPushdown.java @@ -159,6 +159,8 @@ public void testTimestamp64PredicateBoundaries() { String select = "SELECT id FROM " + table.getName(); assertThat(query(select + " WHERE k = " + timestampLiteral(minimum))).matches("VALUES BIGINT '1'").isFullyPushedDown(); assertThat(query(select + " WHERE k = " + timestampLiteral(maximum))).matches("VALUES BIGINT '3'").isFullyPushedDown(); + assertThat(query(select + " WHERE k IN (" + timestampLiteral(minimum) + ", " + timestampLiteral(maximum) + ")")) + .matches("VALUES BIGINT '1', BIGINT '3'").isFullyPushedDown(); assertThat(query(select + " WHERE k >= " + timestampLiteral(minimum))).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'").isFullyPushedDown(); assertThat(query(select + " WHERE k <= " + timestampLiteral(maximum))).matches("VALUES BIGINT '1', BIGINT '2', BIGINT '3'").isFullyPushedDown(); assertThat(query(select + " WHERE k IS NULL")).matches("VALUES BIGINT '4'").isFullyPushedDown();