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

[GitBox <gi...@apache.org>]

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



##########
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:
       > projects
   
   @tristaZero It can be seen from the source code that when the original projects and the optimized projects are the same, projectInts will return null. Maybe the `0 == projects.length` condition is redundant.
   
   ```java
     private static TableScanNode createProjectableFilterable(Compiler compiler,
         TableScan rel, ImmutableList<RexNode> filters, ImmutableIntList projects,
         ProjectableFilterableTable pfTable) {
       final DataContext root = compiler.getDataContext();
       final ImmutableIntList originalProjects = projects;
       for (;;) {
         final List<RexNode> mutableFilters = Lists.newArrayList(filters);
         final int[] projectInts;
         if (projects == null
             || projects.equals(TableScan.identity(rel.getTable()))) {
           projectInts = null;
         } else {
           projectInts = projects.toIntArray();
         }
         final Enumerable<Object[]> enumerable1 =
             pfTable.scan(root, mutableFilters, projectInts);
   ...
   }
   ```

##########
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?
   
   @tristaZero Yes, this does have performance problems, and I will consider caching column names.

##########
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?
   
   I will optimize it.

##########
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?
   
   @tristaZero Good suggestion, I will replace all columns with `*`.




-- 
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