Remove redundent brackets for the join syntax (#649)

* remove redundant brackets

* add relationship test for clickhouse

* enable dynamic-query by default

* fix checkstyle

* fix dry plan test

* remove redundant limit

Author: goldmedal

ibis-server/tests/routers/ibis/test_clickhouse.py

@@ -43,6 +43,24 @@ def clickhouse(request) -> ClickHouseContainer:
         os.path.dirname(__file__), "../../resource/tpch/data/orders.parquet"
     )
     client.insert_df("orders", pd.read_parquet(data_path))
+    client.command("""
+        CREATE TABLE customer (
+            c_custkey        Int32,
+            c_name           String,
+            c_address        String,
+            c_nationkey      Int32,
+            c_phone          String,
+            c_acctbal        Decimal(15,2),
+            c_mktsegment     String,
+            c_comment        String
+        ) 
+        ENGINE = MergeTree
+        ORDER BY (c_custkey)
+    """)
+    data_path = os.path.join(
+        os.path.dirname(__file__), "../../resource/tpch/data/customer.parquet"
+    )
+    client.insert_df("customer", pd.read_parquet(data_path))
     request.addfinalizer(ch.stop)
     return ch
 
@@ -87,9 +105,48 @@ class TestClickHouse:
                         "expression": "toDateTime64('2024-01-01T23:59:59', 9, 'UTC')",
                         "type": "timestamp",
                     },
+                    {
+                        "name": "customer",
+                        "type": "Customer",
+                        "relationship": "OrdersCustomer",
+                    },
+                    {
+                        "name": "customer_name",
+                        "type": "varchar",
+                        "isCalculated": True,
+                        "expression": "customer.name",
+                    },
                 ],
                 "primaryKey": "orderkey",
-            }
+            },
+            {
+                "name": "Customer",
+                "refSql": "select * from test.customer",
+                "columns": [
+                    {"name": "custkey", "expression": "c_custkey", "type": "integer"},
+                    {"name": "name", "expression": "c_name", "type": "varchar"},
+                    {
+                        "name": "orders",
+                        "type": "Orders",
+                        "relationship": "OrdersCustomer",
+                    },
+                    {
+                        "name": "totalprice",
+                        "type": "float",
+                        "isCalculated": True,
+                        "expression": "sum(orders.totalprice)",
+                    },
+                ],
+                "primaryKey": "custkey",
+            },
+        ],
+        "relationships": [
+            {
+                "name": "OrdersCustomer",
+                "models": ["Orders", "Customer"],
+                "joinType": "MANY_TO_ONE",
+                "condition": "Orders.custkey = Customer.custkey",
+            },
         ],
     }
 
@@ -122,7 +179,7 @@ def test_query(self, clickhouse: ClickHouseContainer):
         )
         assert response.status_code == 200
         result = response.json()
-        assert len(result["columns"]) == len(self.manifest["models"][0]["columns"])
+        assert len(result["columns"]) == 9
         assert len(result["data"]) == 1
         assert result["data"][0] == [
             1,
@@ -133,6 +190,7 @@ def test_query(self, clickhouse: ClickHouseContainer):
             "1_370",
             1704153599000,
             1704153599000,
+            "Customer#000000370",
         ]
         assert result["dtypes"] == {
             "orderkey": "int32",
@@ -143,6 +201,7 @@ def test_query(self, clickhouse: ClickHouseContainer):
             "order_cust_key": "object",
             "timestamp": "datetime64[ns]",
             "timestamptz": "datetime64[ns, UTC]",
+            "customer_name": "object",
         }
 
     def test_query_with_connection_url(self, clickhouse: ClickHouseContainer):
@@ -157,7 +216,7 @@ def test_query_with_connection_url(self, clickhouse: ClickHouseContainer):
         )
         assert response.status_code == 200
         result = response.json()
-        assert len(result["columns"]) == len(self.manifest["models"][0]["columns"])
+        assert len(result["columns"]) == 9
         assert len(result["data"]) == 1
         assert result["data"][0][0] == 1
         assert result["dtypes"] is not None
@@ -180,7 +239,7 @@ def test_query_with_column_dtypes(self, clickhouse: ClickHouseContainer):
         )
         assert response.status_code == 200
         result = response.json()
-        assert len(result["columns"]) == len(self.manifest["models"][0]["columns"])
+        assert len(result["columns"]) == 9
         assert len(result["data"]) == 1
         assert result["data"][0] == [
             1,
@@ -191,6 +250,7 @@ def test_query_with_column_dtypes(self, clickhouse: ClickHouseContainer):
             "1_370",
             "2024-01-01 23:59:59.000000",
             "2024-01-01 23:59:59.000000 UTC",
+            "Customer#000000370",
         ]
         assert result["dtypes"] == {
             "orderkey": "int32",
@@ -201,6 +261,7 @@ def test_query_with_column_dtypes(self, clickhouse: ClickHouseContainer):
             "order_cust_key": "object",
             "timestamp": "object",
             "timestamptz": "object",
+            "customer_name": "object",
         }
 
     def test_query_with_limit(self, clickhouse: ClickHouseContainer):
@@ -231,6 +292,63 @@ def test_query_with_limit(self, clickhouse: ClickHouseContainer):
         result = response.json()
         assert len(result["data"]) == 1
 
+    def test_query_join(self, clickhouse: ClickHouseContainer):
+        connection_info = self.to_connection_info(clickhouse)
+        response = client.post(
+            url=f"{self.base_url}/query",
+            json={
+                "connectionInfo": connection_info,
+                "manifestStr": self.manifest_str,
+                "sql": 'SELECT name as customer_name FROM "Orders" join "Customer" on "Orders".custkey = "Customer".custkey WHERE custkey = 370 LIMIT 1',
+            },
+        )
+        assert response.status_code == 200
+        result = response.json()
+        assert len(result["columns"]) == 1
+        assert len(result["data"]) == 1
+        assert result["data"][0] == ["Customer#000000370"]
+        assert result["dtypes"] == {
+            "customer_name": "object",
+        }
+
+    def test_query_to_one_relationship(self, clickhouse: ClickHouseContainer):
+        connection_info = self.to_connection_info(clickhouse)
+        response = client.post(
+            url=f"{self.base_url}/query",
+            json={
+                "connectionInfo": connection_info,
+                "manifestStr": self.manifest_str,
+                "sql": 'SELECT customer_name FROM "Orders" where custkey = 370 LIMIT 1',
+            },
+        )
+        assert response.status_code == 200
+        result = response.json()
+        assert len(result["columns"]) == 1
+        assert len(result["data"]) == 1
+        assert result["data"][0] == ["Customer#000000370"]
+        assert result["dtypes"] == {
+            "customer_name": "object",
+        }
+
+    def test_query_to_many_relationship(self, clickhouse: ClickHouseContainer):
+        connection_info = self.to_connection_info(clickhouse)
+        response = client.post(
+            url=f"{self.base_url}/query",
+            json={
+                "connectionInfo": connection_info,
+                "manifestStr": self.manifest_str,
+                "sql": 'SELECT totalprice FROM "Customer" where custkey = 370 LIMIT 1',
+            },
+        )
+        assert response.status_code == 200
+        result = response.json()
+        assert len(result["columns"]) == 1
+        assert len(result["data"]) == 1
+        assert result["data"][0] == [2860895.79]
+        assert result["dtypes"] == {
+            "totalprice": "object",
+        }
+
     def test_query_without_manifest(self, clickhouse: ClickHouseContainer):
         connection_info = self.to_connection_info(clickhouse)
         response = client.post(

trino-parser/src/main/java/io/trino/sql/SqlFormatter.java

@@ -562,9 +562,6 @@ protected Void visitJoin(Join node, Integer indent)
                 type = "NATURAL " + type;
             }
 
-            if (node.getType() != Join.Type.IMPLICIT) {
-                builder.append('(');
-            }
             process(node.getLeft(), indent);
 
             builder.append('\n');
@@ -594,10 +591,6 @@ else if (!(criteria instanceof NaturalJoin)) {
                 }
             }
 
-            if (node.getType() != Join.Type.IMPLICIT) {
-                builder.append(")");
-            }
-
             return null;
         }
 

trino-parser/src/test/java/io/trino/sql/TestSqlFormatter.java

@@ -0,0 +1,49 @@
+/*
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package io.trino.sql;
+
+import io.trino.sql.parser.ParsingOptions;
+import io.trino.sql.parser.SqlParser;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+public class TestSqlFormatter
+{
+    private static final SqlParser SQL_PARSER = new SqlParser();
+
+    @Test
+    public void testFormatJoin()
+    {
+        String sql = "SELECT * FROM a JOIN b ON a.x = b.y";
+        String formattedSql = SqlFormatter.formatSql(SQL_PARSER.createStatement(sql, new ParsingOptions()));
+        assertEquals("""
+                SELECT *
+                FROM
+                  a
+                INNER JOIN b ON (a.x = b.y)
+                """, formattedSql);
+
+        sql = "SELECT * FROM a JOIN b ON a.x = b.y JOIN c ON a.x = c.z";
+        formattedSql = SqlFormatter.formatSql(SQL_PARSER.createStatement(sql, new ParsingOptions()));
+        assertEquals("""
+                SELECT *
+                FROM
+                  a
+                INNER JOIN b ON (a.x = b.y)
+                INNER JOIN c ON (a.x = c.z)
+                """, formattedSql);
+    }
+}

wren-base/src/main/java/io/wren/base/config/WrenConfig.java

@@ -37,7 +37,7 @@ public enum DataSourceType
 
     private File wrenMDLDirectory = new File("etc/mdl");
     private DataSourceType dataSourceType = DataSourceType.DUCKDB_BACKEND;
-    private boolean enableDynamicFields;
+    private boolean enableDynamicFields = true;
     private String wrenWebUrl = "http://localhost:3000";
 
     @NotNull

wren-main/src/main/java/io/wren/main/PreviewService.java

@@ -30,6 +30,7 @@
 
 import com.google.common.collect.Streams;
 import com.google.inject.Inject;
+import io.airlift.log.Logger;
 import io.wren.base.AnalyzedMDL;
 import io.wren.base.Column;
 import io.wren.base.ConnectorRecordIterator;
@@ -50,6 +51,7 @@
 
 public class PreviewService
 {
+    private static final Logger LOG = Logger.get(PreviewService.class);
     private final Metadata metadata;
 
     private final SqlConverter sqlConverter;
@@ -101,6 +103,7 @@ public CompletableFuture<String> dryPlan(WrenMDL mdl, String sql, boolean isMode
 
             String planned = WrenPlanner.rewrite(sql, sessionContext, new AnalyzedMDL(mdl, null));
             if (isModelingOnly) {
+                LOG.info("Planned SQL: %s", planned);
                 return planned;
             }
             return sqlConverter.convert(planned, sessionContext);

wren-tests/src/test/java/io/wren/testing/TestMDLResource.java

@@ -223,7 +223,7 @@ public void testDryRunAndDryPlan()
                    , "Orders"."custkey" "custkey"
                    , "Orders_relationsub"."customer_name" "customer_name"
                    FROM
-                     ((
+                     (
                       SELECT
                         "Orders"."orderkey" "orderkey"
                       , "Orders"."custkey" "custkey"
@@ -245,7 +245,7 @@ LEFT JOIN (
                         "Orders"."orderkey"
                       , "Customer"."name" "customer_name"
                       FROM
-                        ((
+                        (
                          SELECT
                            o_orderkey "orderkey"
                          , o_custkey "custkey"
@@ -256,8 +256,8 @@ LEFT JOIN (
                               tpch.orders
                          )  "Orders"
                       )  "Orders"
-                      LEFT JOIN "Customer" ON ("Customer"."custkey" = "Orders"."custkey"))
-                   )  "Orders_relationsub" ON ("Orders"."orderkey" = "Orders_relationsub"."orderkey"))
+                      LEFT JOIN "Customer" ON ("Customer"."custkey" = "Orders"."custkey")
+                   )  "Orders_relationsub" ON ("Orders"."orderkey" = "Orders_relationsub"."orderkey")
                 )\s
                 SELECT customer_name
                 FROM

wren-tests/src/test/java/io/wren/testing/TestMDLResourceV2.java

@@ -229,7 +229,7 @@ public void testDryPlan()
                    , "Orders"."custkey" "custkey"
                    , "Orders_relationsub"."customer_name" "customer_name"
                    FROM
-                     ((
+                     (
                       SELECT
                         "Orders"."orderkey" "orderkey"
                       , "Orders"."custkey" "custkey"
@@ -251,7 +251,7 @@ LEFT JOIN (
                         "Orders"."orderkey"
                       , "Customer"."name" "customer_name"
                       FROM
-                        ((
+                        (
                          SELECT
                            o_orderkey "orderkey"
                          , o_custkey "custkey"
@@ -262,8 +262,8 @@ LEFT JOIN (
                               tpch.orders
                          )  "Orders"
                       )  "Orders"
-                      LEFT JOIN "Customer" ON ("Customer"."custkey" = "Orders"."custkey"))
-                   )  "Orders_relationsub" ON ("Orders"."orderkey" = "Orders_relationsub"."orderkey"))
+                      LEFT JOIN "Customer" ON ("Customer"."custkey" = "Orders"."custkey")
+                   )  "Orders_relationsub" ON ("Orders"."orderkey" = "Orders_relationsub"."orderkey")
                 )\s
                 SELECT customer_name
                 FROM