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
[SPARK-21121][SQL] Support changing storage level via the spark.sql.inMemoryColumnarStorage.level variable #18328
Conversation
query: Dataset[_], | ||
tableName: Option[String] = None | ||
): Unit = writeLock { | ||
cacheQuery(query, tableName, StorageLevel.fromString(query.sparkSession.sessionState.conf.cacheStorageLevel)) |
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.
This line exceeds 100 characters.
def cacheQuery( | ||
query: Dataset[_], | ||
tableName: Option[String] = None | ||
): Unit = writeLock { |
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.
Combine the two lines together
Could you add a test case in |
ok to test |
Test build #78187 has finished for PR 18328 at commit
|
…nMemoryColumnarStorage.level variable
Test build #78201 has started for PR 18328 at commit |
Test FAILed. |
@@ -106,6 +105,11 @@ class CacheManager extends Logging { | |||
} | |||
} | |||
|
|||
def cacheQuery(query: Dataset[_], tableName: Option[String] = None): Unit = writeLock { | |||
cacheQuery(query, tableName, StorageLevel.fromString( | |||
query.sparkSession.sessionState.conf.cacheStorageLevel)) |
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.
btw, this is a bit odd to roundtrip to string <-> storagelevel...
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.
Could you explain your question in a bit more details?
@dosoft Is this PR active? then would you mind if I ask to reply to the review comment above? |
What changes were proposed in this pull request?
As described in title
How was this patch tested?