-
Notifications
You must be signed in to change notification settings - Fork 3.3k
Fix column level lineage miss when use unnest #26559
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
@Praveen2112 Could you help review it? thanks |
e3311e1
to
f6b8286
Compare
testing/trino-tests/src/test/java/io/trino/execution/TestEventListenerBasic.java
Outdated
Show resolved
Hide resolved
24a364a
to
c01ecc0
Compare
c01ecc0
to
34a0097
Compare
34a0097
to
34b5b3c
Compare
I think overall is good, but I am not familiar and confident in lineage part. still needs @Praveen2112 to take a look |
new OutputColumnMetadata("test_varchar", VARCHAR_TYPE, ImmutableSet.of(new ColumnDetail("mock", "default", "tests_table_unnest", "test_varchar_array"))), | ||
new OutputColumnMetadata("test_bigint", BIGINT_TYPE, ImmutableSet.of(new ColumnDetail("mock", "default", "tests_table_unnest", "test_bigint")))); | ||
assertLineage( | ||
"SELECT test_varchar_unnest AS test_varchar, test_bigint_unnest AS test_bigint FROM mock.default.tests_table_unnest CROSS JOIN UNNEST(test_varchar_array) WITH ORDINALITY AS t(test_varchar_unnest, test_bigint_unnest)", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What if we if provide t(test_varchar_unnest, test_bigint_unnest, row_number)
and try to assert the lineage of the row number ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If use row number column, this column lineage is empty.
for (Field field : outputFields) { | ||
for (Map.Entry<NodeRef<Expression>, List<Field>> entry : mappings.entrySet()) { | ||
Expression expression = entry.getKey().getNode(); | ||
List<Field> fields = entry.getValue(); | ||
if (fields.contains(field)) { | ||
analysis.addSourceColumns(field, analysis.getExpressionSourceColumns(expression)); | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any specific reason for this for loop
- instead we could add them as process each expression right ? Like
expressionOutputs.forEach(field -> analysis.addSourceColumns(field, analysis.getExpressionSourceColumns(expression)));
And similarly for cardinality as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, only add them as process each expression. Thank you for your feedback!
34b5b3c
to
b6013e0
Compare
b6013e0
to
36f52e1
Compare
@Praveen2112 PTAL, thanks! |
Description
ReOpening #24620 since it went stale.
Fixes #16946
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: