[CALCITE-6744] Support getColumnOrigins for correlate in RelMdColumnOrigins #4109
Conversation
|
There are some CI errors, I'll fix it tomorrow. |
suibianwanwank
force-pushed
the
calcite-6744
branch
2 times, most recently
from
5aaf7c5 to
21c5065
Compare
suibianwanwank
force-pushed
the
calcite-6744
branch
from
21c5065 to
82f28af
Compare
| /** | ||
| * RelColumnOrigin is a data structure describing one of the origins of an | ||
| * output column produced by a relational expression. | ||
| */ | ||
| public class RelColumnOrigin { | ||
| //~ Instance fields -------------------------------------------------------- | ||
|
|
||
| private final RelOptTable originTable; | ||
| private final @Nullable RelOptTable originTable; |
There was a problem hiding this comment.
Ideally this would be implemented using an interface and two subclasses, one for each kind of origin.
Not sure how easy this can be accomplished while keeping compatibility.
There was a problem hiding this comment.
I think this would also be difficult to keep compatible. Another compromise would be to keep the RelColumnOrigin unchanged and return null if the input field's CorField comes from outside the input RelNode.
This way we just need to add a Map to maintain the mapping of CorrelationId, Index to ColumnOrigin, and go top-down to getColumnOrigin.
public @Nullable Set<RelColumnOrigin> getColumnOrigins(Aggregate rel, RelMetadataQuery mq, int iOutputColumn, Map<Pair<CorrelationId, Integer>, ColumnOrigin>) corVarOriginMap) { ... }
| @@ -303,6 +328,17 @@ private static Set<RelColumnOrigin> getMultipleColumns(RexNode rexNode, RelNode | |||
| } | |||
| return null; | |||
| } | |||
|
|
|||
| @Override public Void visitFieldAccess(RexFieldAccess fieldAccess) { | |||
There was a problem hiding this comment.
shouldn't this actually use as origin the table that correlated variable comes from (recursively)?
There was a problem hiding this comment.
I'm not sure I fully understand what you mean. There is a case where the RelNode that calls getColumnOrigin is not the top-level RelNode, so it may not get the original table.
|
| assertThat(correlationId, notNullValue()); | ||
| assertThat(correlationId.getName(), equalTo("$cor1")); | ||
| assertThat(origin.getOriginColumnOrdinal(), equalTo(1)); | ||
| continue; |
There was a problem hiding this comment.
in this case I think that an else is more appropriate.
| final RelMetadataQuery mq = | ||
| new RelMetadataQuery(metadataConfig.getDefaultHandlerProvider()); | ||
| Set<RelColumnOrigin> origins = | ||
| mq.getColumnOrigins(relNode.getInput(0).getInput(1), 0); |
There was a problem hiding this comment.
is this LogicalAggregate? Maybe you can say this in a comment.
| } | ||
| assertThat(origin.getOriginTable(), notNullValue()); | ||
| assertThat(origin.getOriginTable().getQualifiedName().get(2), equalTo("DEPT")); | ||
| assertThat(origin.getOriginColumnOrdinal(), equalTo(1)); |
| CorrelationId correlationId = origin.getCorrelationId(); | ||
| assertThat(correlationId, notNullValue()); | ||
| assertThat(correlationId.getName(), equalTo("$cor1")); | ||
| assertThat(origin.getOriginColumnOrdinal(), equalTo(1)); |
There was a problem hiding this comment.
want to check that originTable is null too?
Is this ENAME?
This PR introduces support for the getOriginColumn method for Correlate.
To represent the origin of the corVar field, the ColumnOrigin class has been modified accordingly. This caused breaking changes, so I'm documenting it in history.md. If there's a better way to do it, welcome any suggestions.