[ https://issues.apache.org/jira/browse/CALCITE-3228?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16899594#comment-16899594 ]
jin xing edited comment on CALCITE-3228 at 8/4/19 9:28 AM: ----------------------------------------------------------- [^CALCITE-3228.patch] Hi [~Chunwei Lei] I try to dig and learn from this issue; *Project::getMapping* try to return an inverse-surjection mapping. *SurjectionWithInverse* is defined as below: {code:java} /** * A surjection with inverse has precisely one source for each target. * (Whereas a general surjection has at least one source for each target.) * Every source has at most one target. */ {code} That's why the test you provided throws {code:java} ... Caused by: java.lang.IllegalArgumentException: source #0 is already mapped to target #1 at org.apache.calcite.util.mapping.Mappings$SurjectionWithInverse.set(Mappings.java:1326) at org.apache.calcite.rel.core.Project.getMapping(Project.java:285) at org.apache.calcite.rel.core.Project.getMapping(Project.java:256) at org.apache.calcite.rel.rules.ProjectTableScanRule.apply(ProjectTableScanRule.java:107) at org.apache.calcite.rel.rules.ProjectTableScanRule$2.onMatch(ProjectTableScanRule.java:83) at org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:208) ... {code} I have two ideas and it's great if you can confirm and rectify when you have time: 1. *ProjectTableScanRule* should not be limited by the Project::getMapping. I mean ProjectTableScanRule can handle to create a *BindableTableScan* as long as *project.getProjects().stream().allMatch(p -> p instanceof RexInputRef) == true,* and no need to be limited by a inverse-surjection mapping. 2. The definition of *Project::getMapping* is as below {code:java} /** * Returns a mapping of a set of project expressions. * * <p>The mapping is an inverse surjection. * Every target has a source field, but * a source field may appear as zero, one, or more target fields. * Thus you can safely call * {@link org.apache.calcite.util.mapping.Mappings.TargetMapping#getTarget(int)}. * {code} But in my understanding, it's not safe to call *getTarget(int)* but safe to call *getSource(int);* I try to file a fix for this issue and illustrate my idea. It's great if you can shepherd and tell if it's the right direction. If you haven't started a PR on this issue, I can work on it under your shepherd. But since you have the expertise, if you have already working on, I will learn and track your PR. was (Author: jinxing6...@126.com): [^CALCITE-3228.patch] Hi [~Chunwei Lei] I try to dig and learn from this issue; *Project::getMapping* try to return an inverse-surjection mapping. *SurjectionWithInverse* is defined as below: {code:java} /** * A surjection with inverse has precisely one source for each target. * (Whereas a general surjection has at least one source for each target.) * Every source has at most one target. */ {code} That's why the test you provided throws {code:java} ... Caused by: java.lang.IllegalArgumentException: source #0 is already mapped to target #1 at org.apache.calcite.util.mapping.Mappings$SurjectionWithInverse.set(Mappings.java:1326) at org.apache.calcite.rel.core.Project.getMapping(Project.java:285) at org.apache.calcite.rel.core.Project.getMapping(Project.java:256) at org.apache.calcite.rel.rules.ProjectTableScanRule.apply(ProjectTableScanRule.java:107) at org.apache.calcite.rel.rules.ProjectTableScanRule$2.onMatch(ProjectTableScanRule.java:83) at org.apache.calcite.plan.volcano.VolcanoRuleCall.onMatch(VolcanoRuleCall.java:208) ... {code} I have two ideas and it's great if you can confirm and rectify when you have time: 1. *ProjectTableScanRule* should not be limited by the Project::getMapping. I mean ProjectTableScanRule can handle to create a BindableTableScan as long as *project.getProjects().stream().allMatch(p -> p instanceof RexInputRef) == true,* and no need to be limited by a inverse-surjection mapping. 2. The definition of *Project::getMapping* is as below {code:java} /** * Returns a mapping of a set of project expressions. * * <p>The mapping is an inverse surjection. * Every target has a source field, but * a source field may appear as zero, one, or more target fields. * Thus you can safely call * {@link org.apache.calcite.util.mapping.Mappings.TargetMapping#getTarget(int)}. * {code} But in my understanding, it's not safe to call *getTarget(int)* but safe to call *getSource(int);* I try to file a fix for this issue and illustrate my idea. It's great if you can shepherd and tell if it's the right direction. If you haven't started a PR on this issue, I can work on it under your shepherd. But since you have the expertise, if you have already working on, I will learn and track your PR. > Error while applying rule ProjectScanRule: interpreter > ------------------------------------------------------ > > Key: CALCITE-3228 > URL: https://issues.apache.org/jira/browse/CALCITE-3228 > Project: Calcite > Issue Type: Bug > Components: core > Reporter: Chunwei Lei > Priority: Critical > Fix For: 1.21.0 > > Attachments: CALCITE-3228.patch > > > The following test can reproduce the issue. > > {code:java} > // FrameworksTest.java > @Test public void testMinMax() throws Exception { > Table table = new TableImpl(); > final SchemaPlus rootSchema = Frameworks.createRootSchema(true); > SchemaPlus schema = rootSchema.add("x", new AbstractSchema()); > schema.add("MYTABLE", table); > List<RelTraitDef> traitDefs = new ArrayList<>(); > traitDefs.add(ConventionTraitDef.INSTANCE); > traitDefs.add(RelDistributionTraitDef.INSTANCE); > SqlParser.Config parserConfig = > SqlParser.configBuilder(SqlParser.Config.DEFAULT) > .setCaseSensitive(false) > .build(); > final FrameworkConfig config = Frameworks.newConfigBuilder() > .parserConfig(parserConfig) > .defaultSchema(schema) > .traitDefs(traitDefs) > // define the rules you want to apply > .ruleSets( > RuleSets.ofList(AbstractConverter.ExpandConversionRule.INSTANCE, > ProjectTableScanRule.INSTANCE)) > .programs(Programs.ofRules(Programs.RULE_SET)) > .build(); > executeQuery(config, " select min(id) as mi, max(id) as ma from mytable where > id=1 group by id", > CalciteSystemProperty.DEBUG.value()); > } > {code} > -- This message was sent by Atlassian JIRA (v7.6.14#76016)