Skip to content

Comments

[CALCITE-6744] RelMetadataQuery.getColumnOrigins should return null when column origin includes correlation variables#4145

Merged
mihaibudiu merged 1 commit intoapache:mainfrom
suibianwanwank:calcite-6744_2
Jan 17, 2025
Merged

[CALCITE-6744] RelMetadataQuery.getColumnOrigins should return null when column origin includes correlation variables#4145
mihaibudiu merged 1 commit intoapache:mainfrom
suibianwanwank:calcite-6744_2

Conversation

@suibianwanwank
Copy link
Contributor

…lation variable

if (inputSet != null) {
set.addAll(inputSet);
if (inputSet == null) {
return null;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic here doesn't seem to fit the documentation.
set of origin columns, or null if this information cannot be determined (whereas empty set indicates definitely no origin columns at all)

return null;
}

@Override public Void visitFieldAccess(RexFieldAccess fieldAccess) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why does this indicate an access in a correlated variable?
could this be an access into a field with type ROW?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, there's a processing problem here. I fixed it.

Copy link
Contributor

@dssysolyatin dssysolyatin Dec 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a bug. What if fieldAccess.getReferenceExpr() is input ref ? For example: 'struct_column.field1.field2'. struct_column is InputRef.

Or:

$cor0.struct_field.field_1

I think implementation should be:

@Override public Void visitFieldAccess(RexFieldAccess fieldAccess) {
      if (fieldAccess.getReferenceExpr() instanceof RexCorrelVariable) {
              throw Util.FoundOne.NULL;
            }
        return super.visitFieldAccess(fieldAccess)
}

Or probably just use

@Override public R visitCorrelVariable(RexCorrelVariable correlVariable) {
    throw Util.FoundOne.NULL;
  }

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dssysolyatin comments on a closed PR will be easy to overlook, if you think there is a problem, please file another JIRA issue.

@mihaibudiu
Copy link
Contributor

PR and commit should match JIRA

@mihaibudiu mihaibudiu added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Jan 16, 2025
…hen column origin includes correlation variables
@sonarqubecloud
Copy link

@suibianwanwank suibianwanwank changed the title [CALCITE-6744] Returns null when the column origin contains the corre… [CALCITE-6744] RelMetadataQuery.getColumnOrigins should return null when column origin includes correlation variables Jan 17, 2025
@suibianwanwank
Copy link
Contributor Author

PR and commit should match JIRA

Done.

@mihaibudiu mihaibudiu merged commit 891ffc4 into apache:main Jan 17, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants