You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/06/18 21:07:03 UTC

[GitHub] [shardingsphere] tristaZero commented on a change in pull request #10848: optimize select group by having with calcite

tristaZero commented on a change in pull request #10848:
URL: https://github.com/apache/shardingsphere/pull/10848#discussion_r654057982



##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/federate/schema/table/generator/FederateExecutionSQLGenerator.java
##########
@@ -35,14 +38,17 @@
     
     private final int[] projects;
     
+    private final List<RelDataTypeField> fields;
+    
     /**
      * Generate sql.
      *
      * @param table table
      * @return sql
      */
     public String generate(final String table) {
-        // TODO
-        return String.format("SELECT * FROM %s", table);
+        // TODO generate sql with filters
+        String project = null == projects || 0 == projects.length ? "*" : Arrays.stream(projects).mapToObj(each -> fields.get(each).getName()).collect(Collectors.joining(", "));

Review comment:
       Which condition will make`project = null == projects || 0 == projects.length`  true?

##########
File path: shardingsphere-infra/shardingsphere-infra-optimize/src/main/java/org/apache/shardingsphere/infra/optimize/core/metadata/FederateTableMetadata.java
##########
@@ -88,4 +91,13 @@ private DataSource getActualDataSource(final Map<String, DataSource> dataSources
         }
         return dataSources.get(result);
     }
+    
+    /**
+     * Get rel data type field.
+     * 
+     * @return rel data type field collection
+     */
+    public List<RelDataTypeField> getRelDataTypeField() {

Review comment:
       I have no idea how long a bunch of calls for `relProtoDataType.apply(TYPE_FACTORY)` will take up. But suppose we just wanna get the list of columns, do you think it will be better to cache these column names here from `TableMetaData` or this function?

##########
File path: shardingsphere-infra/shardingsphere-infra-executor/src/main/java/org/apache/shardingsphere/infra/executor/sql/federate/schema/table/generator/FederateExecutionSQLGenerator.java
##########
@@ -35,14 +37,17 @@
     
     private final int[] projects;
     
+    private final List<String> columnNames;
+    
     /**
      * Generate sql.
      *
      * @param table table
      * @return sql
      */
     public String generate(final String table) {
-        // TODO
-        return String.format("SELECT * FROM %s", table);
+        // TODO generate sql with filters
+        String project = null == projects ? "*" : Arrays.stream(projects).mapToObj(columnNames::get).collect(Collectors.joining(", "));

Review comment:
       Considering `projects` is for SQL optimization, at this moment, we can consider returning the result only with `Arrays.stream(projects).mapToObj(columnNames::get).collect(Collectors.joining(", ")`. How do you think?

##########
File path: shardingsphere-infra/shardingsphere-infra-optimize/src/main/java/org/apache/shardingsphere/infra/optimize/core/metadata/FederateTableMetadata.java
##########
@@ -88,4 +96,13 @@ private DataSource getActualDataSource(final Map<String, DataSource> dataSources
         }
         return dataSources.get(result);
     }
+    
+    /**
+     * Get column names.
+     * 
+     * @return column names collection
+     */
+    public List<String> getColumnNames() {

Review comment:
       Maybe a @Getter is another better option?




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org