Skip to content
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

[Bug] PVM cause cache table not work #5937

Closed
2 of 4 tasks
AngersZhuuuu opened this issue Jan 3, 2024 · 0 comments
Closed
2 of 4 tasks

[Bug] PVM cause cache table not work #5937

AngersZhuuuu opened this issue Jan 3, 2024 · 0 comments
Labels
kind:bug This is a clearly a bug priority:major

Comments

@AngersZhuuuu
Copy link
Contributor

Code of Conduct

Search before asking

  • I have searched in the issues and found no similar issues.

Describe the bug

PVM cause cache table not work

Affects Version(s)

master

Kyuubi Server Log Output

No response

Kyuubi Engine Log Output

No response

Kyuubi Server Configurations

No response

Kyuubi Engine Configurations

No response

Additional context

No response

Are you willing to submit PR?

  • Yes. I would be willing to submit a PR with guidance from the Kyuubi community to fix.
  • No. I cannot submit a PR at this time.
@AngersZhuuuu AngersZhuuuu added kind:bug This is a clearly a bug priority:major labels Jan 3, 2024
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this issue Feb 5, 2024
# 🔍 Description
## Issue References 🔗

This pull request fixes apache#5937

## Describe Your Solution 🔧

If we cache a table with persist view in the query, since cache table use analyzed plan, so in kyuubi authz we will use PVM to wrap the view, but cache table use canonicalized plan, so we need to implement the `doCanonicalize()` method to ignore the impact of PVM, or it will cache cached table can't be matched.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#5982 from AngersZhuuuu/KYUUBI-5937.

Closes apache#5937

e28275f [Angerszhuuuu] Update PermanentViewMarker.scala
c504103 [Angerszhuuuu] Update PermanentViewMarker.scala
19102ff [Angerszhuuuu] [KYUUBI-5937][Bug] PVM cause cache table not work

Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
zhaohehuhu pushed a commit to zhaohehuhu/incubator-kyuubi that referenced this issue Mar 21, 2024
# 🔍 Description
## Issue References 🔗

This pull request fixes apache#5937

## Describe Your Solution 🔧

If we cache a table with persist view in the query, since cache table use analyzed plan, so in kyuubi authz we will use PVM to wrap the view, but cache table use canonicalized plan, so we need to implement the `doCanonicalize()` method to ignore the impact of PVM, or it will cache cached table can't be matched.

## Types of changes 🔖

- [x] Bugfix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)

## Test Plan 🧪

#### Behavior Without This Pull Request ⚰️

#### Behavior With This Pull Request 🎉

#### Related Unit Tests

---

# Checklist 📝

- [ ] This patch was not authored or co-authored using [Generative Tooling](https://www.apache.org/legal/generative-tooling.html)

**Be nice. Be informative.**

Closes apache#5982 from AngersZhuuuu/KYUUBI-5937.

Closes apache#5937

e28275f [Angerszhuuuu] Update PermanentViewMarker.scala
c504103 [Angerszhuuuu] Update PermanentViewMarker.scala
19102ff [Angerszhuuuu] [KYUUBI-5937][Bug] PVM cause cache table not work

Authored-by: Angerszhuuuu <[email protected]>
Signed-off-by: Cheng Pan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug This is a clearly a bug priority:major
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant