[GitHub] [ignite-3] zstan commented on a diff in pull request #2181: IGNITE-19709 Sql. Get rid of starting implicit transactions for SELECT statements without FROM keyword, change mapping

["zstan (via GitHub)" <gi...@apache.org>]

zstan commented on code in PR #2181:
URL: https://github.com/apache/ignite-3/pull/2181#discussion_r1228370294


##########
modules/sql-engine/src/main/java/org/apache/ignite/internal/sql/engine/metadata/IgniteFragmentMapping.java:
##########
@@ -85,33 +231,19 @@ public MetadataDef<FragmentMappingMetadata> getDef() {
      * @param mq  Metadata query instance. Used to request appropriate metadata from node children.
      * @return Nodes mapping, representing a list of nodes capable to execute a query over particular partitions.
      */
-    public FragmentMapping fragmentMapping(RelNode rel, RelMetadataQuery mq, MappingQueryContext ctx) {
-        throw new AssertionError();
-    }
-
-    /**
-     * See {@link IgniteMdFragmentMapping#fragmentMapping(RelNode, RelMetadataQuery, MappingQueryContext)}.
-     */
-    public FragmentMapping fragmentMapping(RelSubset rel, RelMetadataQuery mq, MappingQueryContext ctx) {
-        throw new AssertionError();
-    }
-
-    /**
-     * See {@link IgniteMdFragmentMapping#fragmentMapping(RelNode, RelMetadataQuery, MappingQueryContext)}.
-     */
-    public FragmentMapping fragmentMapping(SingleRel rel, RelMetadataQuery mq, MappingQueryContext ctx) {
+    private static FragmentMapping fragmentMapping(SingleRel rel, RelMetadataQuery mq, MappingQueryContext ctx) {
         return fragmentMappingForMetadataQuery(rel.getInput(), mq, ctx);
     }
 
     /**
-     * See {@link IgniteMdFragmentMapping#fragmentMapping(RelNode, RelMetadataQuery, MappingQueryContext)}.
+     * See {@link IgniteFragmentMapping#fragmentMapping(SingleRel, RelMetadataQuery, MappingQueryContext)}.
      *
      * <p>{@link ColocationMappingException} may be thrown on two children nodes locations merge. This means that the fragment
      * (which part the parent node is) cannot be executed on any node and additional exchange is needed. This case we throw {@link
      * NodeMappingException} with an edge, where we need the additional exchange. After the exchange is put into the fragment and the
      * fragment is split into two ones, fragment meta information will be recalculated for all fragments.
      */
-    public FragmentMapping fragmentMapping(BiRel rel, RelMetadataQuery mq, MappingQueryContext ctx) {
+    private static FragmentMapping fragmentMapping(BiRel rel, RelMetadataQuery mq, MappingQueryContext ctx) {

Review Comment:
   we can inline them only partially, for example 
   `fragmentMapping(BiRel rel` it\`s about CorrJoin, NestedJoin and LoopJoin too



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

To unsubscribe, e-mail: notifications-unsubscribe@ignite.apache.org

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