Skip to content

Conversation

@zhengruifeng
Copy link
Contributor

What changes were proposed in this pull request?

#39925 introduced a new approach to resolve column references with PLAN_ID, this PR tests what will happen if we apply it in regular Spark SQL.

Why are the changes needed?

NA

Does this PR introduce any user-facing change?

NO

How was this patch tested?

CI

Was this patch authored or co-authored using generative AI tooling?

NO

@zhengruifeng zhengruifeng changed the title [TEST ONLY][SQL] Test resolve column reference with PLAN_ID [TEST ONLY][SQL] Test resolve column references with PLAN_ID Sep 26, 2023
@github-actions github-actions bot added the SQL label Sep 26, 2023
zhengruifeng added a commit that referenced this pull request Oct 4, 2023
…cated column"

### What changes were proposed in this pull request?
This reverts commit 73d3c49.

### Why are the changes needed?
to address #42828 (comment) and #43115 (comment), should not use `GetColumnByOrdinal` in this case.

Need to find another approach, but let's revert it first.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
CI

### Was this patch authored or co-authored using generative AI tooling?
No

Closes #43211 from zhengruifeng/revert_SPARK_45088.

Authored-by: Ruifeng Zheng <[email protected]>
Signed-off-by: Ruifeng Zheng <[email protected]>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can still eagerly resolve the column here to make sure it exists, then create an UnresolvedAttribute from the resolved AttributeReference: UnresolvedAttribute(Seq(attr.name)).

LuciferYang pushed a commit to LuciferYang/spark that referenced this pull request Oct 7, 2023
…cated column"

### What changes were proposed in this pull request?
This reverts commit 73d3c49.

### Why are the changes needed?
to address apache#42828 (comment) and apache#43115 (comment), should not use `GetColumnByOrdinal` in this case.

Need to find another approach, but let's revert it first.

### Does this PR introduce _any_ user-facing change?
No

### How was this patch tested?
CI

### Was this patch authored or co-authored using generative AI tooling?
No

Closes apache#43211 from zhengruifeng/revert_SPARK_45088.

Authored-by: Ruifeng Zheng <[email protected]>
Signed-off-by: Ruifeng Zheng <[email protected]>
@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Jan 18, 2024
@github-actions github-actions bot closed this Jan 19, 2024
@cloud-fan cloud-fan removed the Stale label Jan 19, 2024
@cloud-fan cloud-fan reopened this Jan 19, 2024
@cloud-fan
Copy link
Contributor

@zhengruifeng can we rebase and rerun the tests? The new df col framework got several bug fixes and let's try it out again.

@github-actions
Copy link

We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!

@github-actions github-actions bot added the Stale label Apr 29, 2024
@github-actions github-actions bot closed this Apr 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants