[GLUTEN-7087][CH] Support WindowGroupLimitExec#7176
Conversation
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
| local_engine::BroadCastJoinBuilder::init(env); | ||
| local_engine::CacheManager::initJNI(env); | ||
|
|
||
| DB::registerFormats(); |
There was a problem hiding this comment.
Why does this PR need to call DB:: registerFormats()?
There was a problem hiding this comment.
dump block needs formats.
There was a problem hiding this comment.
adding a simple method to dump block contents is very useful for debug
There was a problem hiding this comment.
You mean calling DB:: registerFormats() is for later dumping?
| return out; | ||
| } | ||
|
|
||
| String BlockUtil::dumpBlock(const DB::Block & block) |
There was a problem hiding this comment.
How are about moving to BlockTypeUtils.h/.cpp ?
There was a problem hiding this comment.
BlockTypeUtils.h/.cpp is just about types?
There was a problem hiding this comment.
Actually, it's block and type utils :)
There was a problem hiding this comment.
If dumping is just for debug aid, I think we should move it to DebugUtils.h/cpp, and put it to debug namespace. In product code, we should put it into #ifdef NDEBUG .. #endif. e.g.
#ifdef NDEBUG
debug::dumpxxx();
#endif
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
|
Run Gluten Clickhouse CI |
* support WindowGroupLimit * 0903 * implement window group limit
2 participants
What changes were proposed in this pull request?
(Please fill in changes proposed in this fix)
Fixes: #7087
WindowGroupLimitExecis introduced in spark-3.5. This PR makes it supportedHow was this patch tested?
(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
unit tests
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)