Use ORDER BY tuple() by default in ClickHouse MergeTree

trinodb · Aug 15, 2024 · 49422c5 · 49422c5
1 parent 07dc2f3
commit 49422c5
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 17 deletions.
diff --git a/docs/src/main/sphinx/connector/clickhouse.md b/docs/src/main/sphinx/connector/clickhouse.md
@@ -154,13 +154,13 @@ WITH (
 
 The following are supported ClickHouse table properties from [https://clickhouse.tech/docs/en/engines/table-engines/mergetree-family/mergetree/](https://clickhouse.tech/docs/en/engines/table-engines/mergetree-family/mergetree/)
 
-| Property name  | Default value | Description                                                                                                             |
-| -------------- | ------------- | ----------------------------------------------------------------------------------------------------------------------- |
-| `engine`       | `Log`         | Name and parameters of the engine.                                                                                      |
-| `order_by`     | (none)        | Array of columns or expressions to concatenate to create the sorting key. Required if `engine` is `MergeTree`.          |
-| `partition_by` | (none)        | Array of columns or expressions to use as nested partition keys. Optional.                                              |
-| `primary_key`  | (none)        | Array of columns or expressions to concatenate to create the primary key. Optional.                                     |
-| `sample_by`    | (none)        | An expression to use for [sampling](https://clickhouse.tech/docs/en/sql-reference/statements/select/sample/). Optional. |
+| Property name  | Default value | Description                                                                                                                      |
+| -------------- | ------------- |----------------------------------------------------------------------------------------------------------------------------------|
+| `engine`       | `Log`         | Name and parameters of the engine.                                                                                               |
+| `order_by`     | (none)        | Array of columns or expressions to concatenate to create the sorting key. Use `tuple()` by default when `engine` is `MergeTree`. |
+| `partition_by` | (none)        | Array of columns or expressions to use as nested partition keys. Optional.                                                       |
+| `primary_key`  | (none)        | Array of columns or expressions to concatenate to create the primary key. Optional.                                              |
+| `sample_by`    | (none)        | An expression to use for [sampling](https://clickhouse.tech/docs/en/sql-reference/statements/select/sample/). Optional.          |
 
 Currently the connector only supports `Log` and `MergeTree` table engines
 in create table statement. `ReplicatedMergeTree` engine is not yet supported.

diff --git a/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java b/plugin/trino-clickhouse/src/main/java/io/trino/plugin/clickhouse/ClickHouseClient.java
@@ -157,7 +157,6 @@
 import static io.trino.plugin.jdbc.TypeHandlingJdbcSessionProperties.getUnsupportedTypeHandling;
 import static io.trino.plugin.jdbc.UnsupportedTypeHandling.CONVERT_TO_VARCHAR;
 import static io.trino.spi.StandardErrorCode.GENERIC_INTERNAL_ERROR;
-import static io.trino.spi.StandardErrorCode.INVALID_TABLE_PROPERTY;
 import static io.trino.spi.StandardErrorCode.NOT_SUPPORTED;
 import static io.trino.spi.StandardErrorCode.SCHEMA_NOT_EMPTY;
 import static io.trino.spi.connector.ConnectorMetadata.MODIFYING_ROWS_MESSAGE;
@@ -401,9 +400,8 @@ protected List<String> createTableSqls(RemoteTableName remoteTableName, List<Str
         ClickHouseEngineType engine = ClickHouseTableProperties.getEngine(tableProperties);
         tableOptions.add("ENGINE = " + engine.getEngineType());
         if (engine == ClickHouseEngineType.MERGETREE && formatProperty(ClickHouseTableProperties.getOrderBy(tableProperties)).isEmpty()) {
-            // order_by property is required
-            throw new TrinoException(INVALID_TABLE_PROPERTY,
-                    format("The property of %s is required for table engine %s", ClickHouseTableProperties.ORDER_BY_PROPERTY, engine.getEngineType()));
+            // https://clickhouse.com/docs/en/engines/table-engines/mergetree-family/mergetree#order_by
+            tableOptions.add("ORDER BY tuple()");
         }
         formatProperty(ClickHouseTableProperties.getOrderBy(tableProperties)).ifPresent(value -> tableOptions.add("ORDER BY " + value));
         formatProperty(ClickHouseTableProperties.getPrimaryKey(tableProperties)).ifPresent(value -> tableOptions.add("PRIMARY KEY " + value));

diff --git a/...rino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java b/...rino-clickhouse/src/test/java/io/trino/plugin/clickhouse/TestClickHouseConnectorTest.java
@@ -355,7 +355,9 @@ public void testDifferentEngine()
         assertThat(getQueryRunner().tableExists(getSession(), tableName)).isTrue();
         assertUpdate("DROP TABLE " + tableName);
         // MergeTree without order by
-        assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree')", "The property of order_by is required for table engine MergeTree\\(\\)");
+        assertUpdate("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'MergeTree')");
+        assertThat(getQueryRunner().tableExists(getSession(), tableName)).isTrue();
+        assertUpdate("DROP TABLE " + tableName);
 
         // MergeTree with optional
         assertUpdate("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR, logdate DATE NOT NULL) WITH " +
@@ -373,7 +375,7 @@ public void testDifferentEngine()
 
         //NOT support engine
         assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x VARCHAR) WITH (engine = 'bad_engine')",
-                "line 1:76: Unable to set catalog 'clickhouse' table property 'engine' to.*");
+                ".* Unable to set catalog 'clickhouse' table property 'engine' to.*");
     }
 
     @Test
@@ -509,15 +511,15 @@ public void testTableProperty()
 
         // Primary key must be a prefix of the sorting key,
         assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL, x boolean NOT NULL, y boolean NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['id'], sample_by = ARRAY['x', 'y'])",
-                "line 1:151: Invalid value for catalog 'clickhouse' table property 'sample_by': .*");
+                ".* Invalid value for catalog 'clickhouse' table property 'sample_by': .*");
 
         // wrong property type
         assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL) WITH (engine = 'MergeTree', order_by = 'id')",
-                "line 1:87: Invalid value for catalog 'clickhouse' table property 'order_by': .*");
+                ".* Invalid value for catalog 'clickhouse' table property 'order_by': .*");
         assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['id'], primary_key = 'id')",
-                "line 1:111: Invalid value for catalog 'clickhouse' table property 'primary_key': .*");
+                ".* Invalid value for catalog 'clickhouse' table property 'primary_key': .*");
         assertQueryFails("CREATE TABLE " + tableName + " (id int NOT NULL) WITH (engine = 'MergeTree', order_by = ARRAY['id'], primary_key = ARRAY['id'], partition_by = 'id')",
-                "line 1:138: Invalid value for catalog 'clickhouse' table property 'partition_by': .*");
+                ".* Invalid value for catalog 'clickhouse' table property 'partition_by': .*");
     }
 
     @Test