IGNITE-28556 Calcite. Support wrap_key and wrap_value flags by zstan · Pull Request #13037 · apache/ignite

zstan · 2026-04-16T07:11:37Z

No description provided.

Copilot

Pull request overview

Adds Calcite-side support for wrap_key / wrap_value CREATE TABLE options (parsing, command propagation, and validation), plus tests to verify wrapping behavior and related DDL constraints.

Changes:

Extend Calcite CREATE TABLE option set with WRAP_KEY and WRAP_VALUE and parse them into CreateTableCommand.
Add DDL-time validation for invalid wrap combinations and implement wrap_value=false mapping to a “flat” value field.
Add/adjust JDBC and integration tests covering wrapping combinations and error scenarios.

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/H2DynamicTableSelfTest.java	Minor test Callable signature adjustment.
modules/indexing/src/test/java/org/apache/ignite/internal/processors/cache/index/DynamicColumnsAbstractTest.java	Minor test Callable signature adjustment.
modules/indexing/src/main/java/org/apache/ignite/internal/processors/query/h2/sql/GridSqlCreateTable.java	Javadoc punctuation fix.
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/jdbc/JdbcQueryTest.java	Adds JDBC tests for wrapped/unwrapped key/value behavior (incl. UUID).
modules/calcite/src/test/java/org/apache/ignite/internal/processors/query/calcite/integration/TableDdlIntegrationTest.java	Adds integration tests for wrap options and constraints; re-enables an ALTER TABLE test.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/sql/IgniteSqlCreateTableOptionEnum.java	Adds `WRAP_KEY` / `WRAP_VALUE` option enums.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/DdlSqlToCommandConverter.java	Adds parsing/validation for wrap options.
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/prepare/ddl/CreateTableCommand.java	Adds wrap flags to the DDL command model (defaults: key=false, value=true).
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/ddl/DdlCommandHandler.java	Validates wrap flags and applies `wrap_value=false` mapping; updates ALTER TABLE ADD error message.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

sonarqubecloud · 2026-04-17T07:39:44Z

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

alex-plekhanov · 2026-04-17T06:31:00Z

+                throwOptionParsingException(opt, SIMPLE_PREDICATE, ctx.query());
+
+            String simple = ((SqlIdentifier)val).getSimple().toLowerCase(Locale.ROOT);
+            if (!"true".equals(simple) && !"false".equals(simple))


equalsIgnoreCase?

alex-plekhanov · 2026-04-17T06:31:39Z

    private String paramIsSqlIdentifierValidator(IgniteSqlCreateTableOption opt, PlanningContext ctx) {
        if (!(opt.value() instanceof SqlIdentifier) || !((SqlIdentifier)opt.value()).isSimple())
-            throwOptionParsingException(opt, "a simple identifier", ctx.query());
+            throwOptionParsingException(opt, SIMPLE_PREDICATE, ctx.query());


This method is not used, but can be used to simplify VALUE_IS_IDENTIFIER_VALIDATOR and VALUE_IS_BOOL_IDENTIFIER_VALIDATOR

alex-plekhanov · 2026-04-17T06:41:18Z

+    private Boolean wrapKey;
+
+    /** Wrap value flag. */
+    private boolean wrapValue = true;


Maybe we should use Boolean here and move default logic to command handler? This class is a representation of parsed command, if WRAP_VALUE is not provided it's more correct to see null here.
Let's also add the same comment as for H2 somewhere:
By default value is always wrapped to allow for ALTER TABLE ADD COLUMN commands.

alex-plekhanov · 2026-04-17T09:58:31Z

+
+    /** */
+    @SuppressWarnings("ThrowableNotThrown")
+    private static void assertThrowsSqlException(Callable<?> call, @Nullable String msg) {


We have assertThrows(sql, cls, msg) in parent class, why do we need yet another method?

alex-plekhanov · 2026-04-17T10:00:52Z

+     */
+    @Test
+    public void testAlterTableFailOnSingleCacheValue() {
+        CacheConfiguration c =


Add generics

alex-plekhanov · 2026-04-17T10:08:11Z

            res = new QueryEntityEx(res).implicitPk(true);
        }

+        if (!cmd.wrapValue()) {


wrapKey is not handled. Only validated.

alex-plekhanov · 2026-04-17T10:11:15Z

+    public void testInlineSizeNoWrap() {
+        try {
+            sql("CREATE TABLE IF NOT EXISTS T ( " +
+                "  id varchar(15), " +
+                "  col varchar(100), " +
+                "  PRIMARY KEY(id) ) ");
+            assertEquals(18, sql(
+                "select INLINE_SIZE from SYS.INDEXES where TABLE_NAME = 'T' and IS_PK = true").get(0).get(0));
+        }
+        finally {
+            sql("DROP TABLE IF EXISTS T");
+        }
+    }
+
+    /**
+     * Test single column PK with wrapping calculate correct inline size.
+     */
+    @Test
+    public void testInlineSizeWrap() {
+        try {
+            sql("CREATE TABLE IF NOT EXISTS T ( " +
+                "  id varchar(15), " +
+                "  col varchar(100), " +
+                "  PRIMARY KEY(id) )  WITH \"wrap_key=true\"");
+            assertEquals(18, sql(
+                "select INLINE_SIZE from SYS.INDEXES where TABLE_NAME = 'T' and IS_PK = true").get(0).get(0));
+        }
+        finally {
+            sql("DROP TABLE IF EXISTS T");
+        }
+    }
+
+    /**
+     * Test two column PK with wrapping calculate correct inline size.
+     */
+    @Test
+    public void testInlineSizeWrapMultiPk() {
+        try {
+            sql("CREATE TABLE IF NOT EXISTS T ( " +
+                "  id varchar(15), " +
+                "  id2 uuid, " +
+                "  col varchar(100), " +
+                "  PRIMARY KEY(id, id2) )  WITH \"wrap_key=true\"");
+            assertEquals(35, sql(
+                "select INLINE_SIZE from SYS.INDEXES where TABLE_NAME = 'T' and IS_PK = true").get(0).get(0));
+        }
+        finally {
+            sql("DROP TABLE IF EXISTS T");
+        }
+    }


It's not a correct tests for wrapping. PK index is always unwrapped if table was created via DDL. You can change wrap_key to false in all these test and get the same result. To test wrap_key correctly you can iterate over cache values and check key, if it primitive type or binary object.

alex-plekhanov · 2026-04-17T10:17:57Z

+     * @param wrapVal Whether value wrap should be enforced.
+     * @throws SQLException if failed.
+     */
+    private void doTestKeyValueWrap(boolean wrapKey, boolean wrapVal, boolean testUuid) throws SQLException {


Looks like it's not related to JDBC somehow, just ordinal integration test.

alex-plekhanov · 2026-04-17T10:18:57Z

+
+            UUID guid = UUID.randomUUID();
+
+            if (wrapKey)


Case with wrap_key=true and without key_type is not covered (and has errors, since wrap_key=true never handled)

zstan requested a review from Copilot April 16, 2026 07:11

Copilot started reviewing on behalf of zstan April 16, 2026 07:12 View session

Copilot AI reviewed Apr 16, 2026

View reviewed changes

zstan force-pushed the ignite-28556 branch 3 times, most recently from f375220 to abd690c Compare April 16, 2026 08:49

IGNITE-28556 Calcite. Support wrap_key and wrap_value flags

284ec53

zstan force-pushed the ignite-28556 branch from abd690c to 284ec53 Compare April 16, 2026 09:01

zstan added 2 commits April 17, 2026 08:17

minor

2c39fbb

support alternate syntax

9b4188d

alex-plekhanov reviewed Apr 17, 2026

View reviewed changes

Conversation

zstan commented Apr 16, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Apr 17, 2026

Quality Gate passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants